MoonModules / WLED

Control WS2812B and many more types of digital RGB LEDs with an ESP8266 or ESP32 over WiFi! MoonModules adds features on top of upstream.
https://mm.kno.wled.ge
GNU General Public License v3.0
202 stars 64 forks source link

Global I2C pin setting being overwritten #21

Open scottrbailey opened 1 year ago

scottrbailey commented 1 year ago

What happened?

Saving changes on the 4LineDisplay config page overwrites the global I2C pin settings when "use global" option is used. Even though the correct pins are shown in "use global" drop down, before the save.

Rebooting after setting global I2C pins and before saving 4LineDisplay page does not make a difference. This bug doesn't impact upstream because all settings are on the same page.

And while your in the code here, maybe you could swap the order of the inputs so they match the order on Usermods page. Usermods (Data, Clock), 4LineDisplay (Clock, Data).

To Reproduce Bug

Expected Behavior

global I2C settings should be preserved.

Install Method

From srg74 firmware repository

What version/release of MM WLED?

WLEDMM_0.14.0-b1.18_esp32_4M_max

Which microcontroller/board are you seeing the problem on?

ESP32

Relevant log/trace output

No response

Anything else?

No response

Code of Conduct

ewoudwijma commented 1 year ago

WLEDMM_0.14.0-b1.18_esp32_4MB_max is full of usermods, where each of the usermods can hijack global pins. It is work in progress to solve this. But this issue helps to prioritize this.

for the time being, try how WLEDMM_0.14.0-b1.18_esp32_4MB_max behaves, it has less usermods, and I think it works better here.

troyhacks commented 1 year ago

I agree with both of these issues and have experienced the same - including the data/clock order confusion.

I'll take a look at the code and see if there's any quick wins. The clock/data (and the labeling of them) should be a minor change.

Other than that, I think we're into "pin registry" improvements.

scottrbailey commented 1 year ago

for the time being, try how WLEDMM_0.14.0-b1.18_esp32_4MB_max behaves

Do you mean WLEDMM_0.14.0-b1.18_esp32_4MB_min or a different version?

ewoudwijma commented 1 year ago

No _max, see also https://mm.kno.wled.ge/moonmodules/Installing-and-Compiling/

_min, _max and _all is little confusing, might update that to something more clear in the future

scottrbailey commented 1 year ago

Oh, well that was the version that I was running. So I'm playing one of those "spot the difference" games trying to figure out what you mean :)

I think any version with the 4LineDisplay mod enabled is going to exhibit that behavior. The settings page for that mod should not modify the value of hw>if>i2c-pin, only um>4LineDisplay>pin.

softhack007 commented 1 year ago

The settings page for that mod should not modify the value of hw>if>i2c-pin, only um>4LineDisplay>pin.

Yes that look like a bug. I'll take a look at it next week, when I'm back from holidays. Also the "swapped order" should be simple to solve - you're right the normal order is SDA, SCL.

softhack007 commented 1 year ago

Current status: -pin assignment: fixed -order of SDA/SCL fields: to be fixed