ESP32: IDF v4.4.3 update + wifi.sta module improvements

nodemcu / nodemcu-firmware

Lua based interactive firmware for ESP8266, ESP8285 and ESP32

https://nodemcu.readthedocs.io

MIT License

7.65k stars 3.12k forks source link

ESP32: IDF v4.4.3 update + wifi.sta module improvements #3564

Closed jmattsson closed 1 year ago

jmattsson commented 1 year ago

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

This PR includes the upgrade to IDF v4.4.3 which we have been using at $work for some time now without issues. To go with it are improvements to the wifi.sta module that expose the new functionality provided by the IDF. Relevant documentation has been updated accordingly.

I expect this will be the last IDF v4 update. I'm hoping to work on getting us ready for IDF v5 (which is now the stable version) either over the holiday break or early in the new year.

Marcel, I tagged you for the docs review predominantly.

marcelstoer commented 1 year ago

Thanks Johny. I'd prefer if the parameters sort_by and scan_method were actual enums rather than enums dressed up as strings.

Update: I guess rather than "enums" I actually meant "constants", sorry.

pjsg commented 1 year ago

Should we be using the value 1 to enable a feature rather than true?

jmattsson commented 1 year ago

You're both raising interesting points. Some time ago somebody pointed out that we let too much "C" into our Lua, and that the Lua canonical way of expression options is as strings, not numbers. Since whoever (and wherever, I can't recall right now) actually provided a reference (last sentence) to the official Lua docs backing up that claim, I've been trying to follow it.

When it comes to 1 vs true, in my case I'd say it's both a case of "C" and age showing through - I learned Lua before there was an actual boolean type, so 1/nil is what I default to when I'm not paying attention. I could certainly be convinced that we should adopt proper boolean types everywhere (possibly with legacy support for 0/1?). In fact, if we do settle on something, it'd be good to get it into the docs proper once and for all.

Thoughts?

jmattsson commented 1 year ago

Okay, I've taken a stab at it and added a small blurb about boolean/toggles to our LRM doc, and added a convenience function to easily be backwards compatible. What do you think?

jmattsson commented 1 year ago

If there's no other feedback I'll merge this Real Soon Now then.

I'm currently doing an IDFv4->IDFv5 migration at $work, which I'll look at PR'ing in the coming week(s). That one will need considerably more testing before I'll feel comfortable merging.