ttlappalainen / NMEA2000_esp32

Inherited object for use NMEA2000 with ESP32 boards
68 stars 41 forks source link

add ESP-IDF v5 support #18

Closed phatpaul closed 1 year ago

phatpaul commented 1 year ago

continuation of PR #15 Tested on esp32 arduino core1 and core2, and on esp-idf 4.4 and 5.0.

You can see the CI build passing here: https://github.com/phatpaul/NMEA2000/tree/phatpaul-ci and here: https://github.com/phatpaul/NMEA2000_switchbank_example_esp-idf

nicklasb commented 1 year ago

@ttlappalainen, @phatpaul

This works for me as well.

ttlappalainen commented 1 year ago

Where are these includes required?

include "rom/gpio.h"

include "soc/gpio_sig_map.h"

include "soc/dport_access.h"

They does not exist on old version and that compiles fine on PlatformIO

ttlappalainen commented 1 year ago

include "esp_idf_version.h" is not available in Arduino IDE 1.8.x

phatpaul commented 1 year ago

I'm going to withdraw this PR. I don't have time to test this old code with old frameworks. I've personally switched to another implementation using the twai api. https://github.com/jiauka/NMEA2000_esp32xx

I recommend adding a note in the documentation about this library not support IDF 5 (still usable for older frameworks) and provide links to some newer alternates: https://github.com/jiauka/NMEA2000_esp32xx https://github.com/sergei/NMEA2000_esp32_twai

nicklasb commented 1 year ago

@ttlappalainen If some files aren't needed, I suppose they can just be deleted.

include "esp_idf_version.h" is not available in Arduino IDE 1.8.x

The 1.8.x branch has not been supported by the Arduino team for a long while. It is not clear to me why a compatibility layer for ESP32 should support a non-supported version of Arduino that AFAIK didn't really support ESP32 at the time? It doesn't have to be as backward-compatible as the main library?

@phatpaul

I'm going to withdraw this PR. I don't have time to test this old code with old frameworks. I've personally switched to another implementation using the twai api.

I hear your frustration and is sad to hear that, personally I like that this library supports both Arduino and ESP-IDF.

ttlappalainen commented 1 year ago

Arduino 1.8.x still works with ESP32. All old projects done with that does not work directly just by moving to 2.x. If possible I prefere to keep backward compatibility for beginners, who has managed to get something running. For them it is sometimes hard job to go latest version with thousands of strange error messages.

As I mentioned somewhere else I have not yet had time to test twai versions. Current version is tested by NMEA 2000 certification.

nicklasb commented 1 year ago

To be honest, I would disagree. It is not my experience that beginners use old versions of frameworks, rather that is the more experienced with a lot of working code that they don't want to make a lot of changes in. And you stop being a beginner not long after your code starts working.

As 2.x is a major release, it is certainly breaking, but staying on a release long after it has stopped being supported by developers is really bad, especially for beginners. If anyone needs to learn that, it is the beginners.

Either way, if this library's not going to work with ESP-IDF 5 after this long, it is basically like not supporting ESP-IDF at all in my view, and thus I would have to agree with @phatpaul on recommendations.