search

nodemcu / nodemcu-firmware

Lua based interactive firmware for ESP8266, ESP8285 and ESP32
https://nodemcu.readthedocs.io
MIT License
7.62k stars 3.12k forks source link

ESP32: Upgrade to IDFv5 #3569

Closed jmattsson closed 5 months ago

jmattsson commented 1 year ago

Rationale

I'm opening this PR primarily as a discussion thread at this point. I've done the grunt work of the upgrade to IDFv5.0 and we have been using this at $work since the start of Feb without any issues. However, that only tests a limited feature set, and I'd like considerable more testing & feedback before merging in this upgrade. For that reason I've pushed this dev-esp32-idf5-testing branch to the main repo rather than leaving it on my own/$work repo alone.

When it comes time to merge this, I propose we cut a dev-esp32-idf4-final branch for those who rely on functionality that's no longer available.

>>> There are a few breaking changes <<<

As this is a major version number change in the IDF, there are a number of breaking changes, some of which inevitably bleed through to the exposed Lua api:

>>> A number of used peripheral APIs are now deprecated <<<

Due to driver rearchitecture and improvements in the IDF, a number of the driver APIs NodeMCU uses are now deprecated and will be removed in a future IDF upgrade. As such, we will need to migrate our platform & module code accordingly. Most of these are ones I have no experience with or easy testability of, so I'd love assistance from the original authors (or others who want to step up). Specifically:

The highest priority here in my view is the RMT driver, as I had to pull in some explicit register definitions to keep NodeMCU building, and I am not sure whether it is still safe to do these direct register manipulations.

I have left all the warnings enabled for the legacy drivers, so their eventual removal does not come as an unexpected shock down the line if we fail to update our code in time.

So, here's my plea to you - "help me all-you-one can-code'rs, you're my only hope" ;-) And as I mentioned at the start, any testing of the IDFv5 branch would be appreciated!

Useful links

pjsg commented 1 year ago

I'll definitely take a look at this. I'm playing with the rmt at the moment.

Philip

On Thu, Feb 9, 2023 at 1:47 AM Johny Mattsson @.***> wrote:

  • This PR is for the dev branch rather than for the release branch.
  • This PR is compliant with the other contributing guidelines http:///CONTRIBUTING.md as well (if not, please describe why).
  • I have thoroughly tested my contribution.
  • The code changes are reflected in the documentation at docs/*.

Rationale

I'm opening this PR primarily as a discussion thread at this point. I've done the grunt work of the upgrade to IDFv5.0 and we have been using this at $work since the start of Feb without any issues. However, that only tests a limited feature set, and I'd like considerable more testing & feedback before merging in this upgrade. For that reason I've pushed this dev-esp32-idf5-testing branch to the main repo rather than leaving it on my own/$work repo alone.

When it comes time to merge this, I propose we cut a dev-esp32-idf4-final branch for those who rely on functionality that's no longer available.

There are a few breaking changes <<<

As this is a major version number change in the IDF, there are a number of breaking changes, some of which inevitably bleed through to the exposed Lua api:

  • The ADC hall effect sensor is no longer readable. The IDF no longer provides an API to access it.
  • FAT partition selection on external SD cards is no longer possible; only the first FAT partition is available. NodeMCU used a bit of a sneaky hack earlier in order to facilitate partition selection, but that avenue has now been closed.
  • When using the sdmmc module, it is now necessary to first configure the SPI master via the spimaster module. The old all-in-one functions we relied upon earlier have been removed. On the bright side, it means there is now a much cleaner delineation between host/device configuration. Arguably we should've gone down this path in the first place.

A number of used peripheral APIs are now deprecated <<<

Due to driver rearchitecture and improvements in the IDF, a number of the driver APIs NodeMCU uses are now deprecated and will be removed in a future IDF upgrade. As such, we will need to migrate our platform & module code accordingly. Most of these are ones I have no experience with or easy testability of, so I'd love assistance from the original authors (or others who want to step up). Specifically:

The highest priority here in my view is the RMT driver, as I had to pull in some explicit register definitions to keep NodeMCU building, and I am not sure whether it is still safe to do these direct register manipulations.

I have left all the warnings enabled for the legacy drivers, so their eventual removal does not come as an unexpected shock down the line if we fail to update our code in time.

So, here's my plea to you - "help me all-you-one can-code'rs, you're my only hope" ;-) And as I mentioned at the start, any testing of the IDFv5 branch would be appreciated! Useful links

You can view, comment on, or merge this pull request online at:

https://github.com/nodemcu/nodemcu-firmware/pull/3569 Commit Summary

File Changes

(45 files https://github.com/nodemcu/nodemcu-firmware/pull/3569/files)

Patch Links:

— Reply to this email directly, view it on GitHub https://github.com/nodemcu/nodemcu-firmware/pull/3569, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALQLTPVAPXTQYGS3E3T3W3WWSHIFANCNFSM6AAAAAAUWE7ZI4 . You are receiving this because you were mentioned.Message ID: @.***>

jmattsson commented 1 year ago

Excellent! Just let me know if you need any input from me!

jmattsson commented 1 year ago

I've upgraded the IDF to v5.0.1, which from a NodeMCU perspective should be an utterly boring upgrade.

tomsci commented 1 year ago

The highest priority here in my view is the RMT driver, as I had to pull in some explicit register definitions to keep NodeMCU building, and I am not sure whether it is still safe to do these direct register manipulations.

Almost certainly not :) https://github.com/nodemcu/nodemcu-firmware/pull/3601 fixes the ws2812 driver's use of RMT although I haven't looked at the rmt module itself.

transmitting by RMT channel doesn’t expect user to prepare the RMT symbols, instead, user needs to provide an RMT Encoder to tell the driver how to convert user data into RMT symbols.

Well that's what my PR does (for ws2812), so that should make the migration a bit easier.

jmattsson commented 1 year ago

@tomsci I would be very happy indeed to see one or more PRs for the RMT things on the IDFv5 branch!

jmattsson commented 1 year ago

@tomsci I've merged in your recent fixes from the IDFv4 branch. If you have a bit of time, could you please check that I resolved the merge conflicts properly?

jmattsson commented 1 year ago

With @tomsci having taken care of my main concern on the IDFv5 upgrade (direct RMT access), I'm leaning towards going ahead with freezing the IDFv4 branch and making this the active dev-esp32 branch. I do not have spare time to effectively maintain two ESP32 branches as PRs land, and the IDF itself is up to 5.0.2 now, so I'm not expecting any bad bugs lingering there at this point.

Thoughts and/or objections anyone? @pjsg @marcelstoer?

marcelstoer commented 1 year ago

No objections from my side.

serg3295 commented 1 year ago

The MQTT module was broken in version 5.0 I have opened PR #3607 that fixes the bug.

pjsg commented 1 year ago

I'm happy to move forward. I'll rebase some other PRs onto the new branch and test them....