Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ESP32: Upgrade to IDFv5 #3569

Merged
merged 7 commits into from
Jan 30, 2024
Merged

ESP32: Upgrade to IDFv5 #3569

merged 7 commits into from
Jan 30, 2024

Conversation

jmattsson
Copy link
Member

  • This PR is for the dev branch rather than for the release branch.
  • This PR is compliant with the other contributing guidelines 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:

  • I2S - used in the i2s module (@devsaurus)
  • RMT - used by the rmt module (@pjsg), and platform component (rmt, ws2812, onewire)
  • ADC - used by the adc module (@zelll), and platform component
  • PCNT - used by the pulsecnt module (@chilipeppr)
  • SIGMADELTA - used by sigmadelta module (@devsaurus) (via the platform component)

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
Copy link
Member

pjsg commented Feb 9, 2023 via email

@jmattsson
Copy link
Member Author

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

@jmattsson
Copy link
Member Author

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

@tomsci
Copy link

tomsci commented May 1, 2023

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 :) #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
Copy link
Member Author

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

@jmattsson
Copy link
Member Author

@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
Copy link
Member Author

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
Copy link
Member

No objections from my side.

@serg3295
Copy link

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

@pjsg
Copy link
Member

pjsg commented Jul 16, 2023

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

jmattsson and others added 7 commits January 30, 2024 11:24
@jmattsson
Initial migration to IDFv5.0
eb6356f 
Plenty of dependency adjustments, printf format specificier updates,
FreeRTOS type and macro name modernisation, not to mention API changes.

Still plenty of legacy/deprecated drivers in use which will need updating.

The following features have been removed due to no longer being available
from the IDF:
  - ADC hall effect sensor reading
  - Configuration of SD SPI host via sdmmc module (now must be done first
    via the spimaster module)
  - FAT partition selection on external SD cards; only the first FAT
    partition is supported by the IDF now

On the other hand, the eth module now supports the following new chipsets:
  - KSZ8001
  - KSZ8021
  - KSZ8031
  - KSZ8051
  - KSZ8061
  - KSZ8091
  - Possibly additional models in the LAN87xx series (the IDF docs aren't
    clear on precisely which models are handled)

Further, the sdmmc module is now available on the ESP32-S3 as well.
@jmattsson
Remove old cryptography package lock.
12f0463 
This workaround is no longer needed nor desired.
@jmattsson
Upgrade to IDF v5.0.2
b536736 
With minor update required to our spi module.
@serg3295 @jmattsson
fix: mqtt module
47ea2bb
@serg3295 @jmattsson
fix(mqtt): select next parameter in connect()
fb72b54
@serg3295 @jmattsson
fix(mqtt): adds parameter checking to nil
f725786
@serg3295 @jmattsson
Fix time.get() for 5.3-int32-singlefp firmware on IDF v5.0.2
ca5acdb
@jmattsson jmattsson merged commit 796fd7a into dev-esp32 Jan 30, 2024
32 checks passed
@jmattsson jmattsson deleted the dev-esp32-idf5-testing branch January 30, 2024 00:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@klacus007 klacus007 klacus007 left review comments

Assignees
No one assigned
Labels
breaking change ESP32
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@jmattsson @pjsg @tomsci @marcelstoer @serg3295 @klacus007