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.

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

Global I2C pin setting being overwritten #21

Open
1 task done
scottrbailey opened this issue Feb 17, 2023 · 7 comments
Open
1 task done

Global I2C pin setting being overwritten #21

scottrbailey opened this issue Feb 17, 2023 · 7 comments
Assignees
Labels
bug Something isn't working fxed in mdev fixed in latest mdev source code

Comments

@scottrbailey
Copy link

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

  • set global I2C pins on usermods page
  • save 4LineDisplay config page

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

  • I agree to follow this project's Code of Conduct
@scottrbailey scottrbailey added the bug Something isn't working label Feb 17, 2023
@ewoudwijma
Copy link
Collaborator

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

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

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

ewoudwijma commented Feb 17, 2023

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

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

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 added a commit that referenced this issue Mar 8, 2023
set.cpp: introduce "-2" => "no value" in addition to "-1" => disable
4LD_ALT:  do not copy "global" pins into UM specific setting; just use them directly.
@softhack007 softhack007 added the fxed in mdev fixed in latest mdev source code label Mar 20, 2023
@softhack007
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fxed in mdev fixed in latest mdev source code
Projects
None yet
Development

No branches or pull requests

4 participants