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

Usermod Four Line Display: unnecessary use of software I2C on ESP32 #2708

Closed
1 task done
softhack007 opened this issue Jul 4, 2022 · 10 comments
Closed
1 task done
Labels

Comments

@softhack007
Copy link
Collaborator

softhack007 commented Jul 4, 2022

What happened?

This is more a performance optimization that a real bug.

On ESP32, any GPIO can be assigned to the I2C hardware driver - ESP32 has two I2C hardware units that could be used in parallel. In the setup functions of four_line_display and four_line_display_ALT, only GPIO 21+22 are accepted as "hardware". If you configure other GPIO pins, a software-only driver is requested from U8g2.

As a result of using software-only I2C, the display will respond slower and there might be delays in animations. In debug build, you can see that a lot of time is spent in the usermod:

  • software I2C: UM time[ms]: 0/196
  • hardware I2C:UM time[ms]: 0/45

To Reproduce Bug

Compile with the following additional build_flags: -D USERMOD_FOUR_LINE_DISPLAY -D FLD_PIN_SCL=4 -D FLD_PIN_SDA=0, --> notice the speed of display updates.

Then compile again with -D FLD_PIN_SCL=22 -D FLD_PIN_SDA=21, and compare performances.

Expected Behavior

Hardware I2C drivers should always be used on ESP32, no matter what GPIO.

Install Method

Self-Compiled

What version of WLED?

WLED main development branch

Which microcontroller/board are you seeing the problem on?

ESP32

Relevant log/trace output

none

Anything else?

As a quick hack that enforces HW i2C drivers:

below
https://github.com/Aircoookie/WLED/blob/a8908238d5e8c0aafb2f603168a193b86338e169/usermods/usermod_v2_four_line_display_ALT/usermod_v2_four_line_display_ALT.h#L211-L212

add this

#ifdef ARDUINO_ARCH_ESP32
    isHW = true;
    po = PinOwner::HW_I2C;
#endif

.

PS: overlooked one ... also this line needs to be patched:
https://github.com/Aircoookie/WLED/blob/a8908238d5e8c0aafb2f603168a193b86338e169/usermods/usermod_v2_four_line_display_ALT/usermod_v2_four_line_display_ALT.h#L214

Code of Conduct

  • I agree to follow this project's Code of Conduct
@softhack007 softhack007 added the bug label Jul 4, 2022
@blazoncek
Copy link
Collaborator

Currently there is no standard way to assign the same I2C pins to multiple usermods.
If this is implemented in core WLED then all usermods could request access to system I2C bus and the problem would dissapear.

I have already thought about that and adjusted pin manager class but then focused elsewhere so the global I2C remained unimplemented. This is due to the fact that all usermods would need to be revised and changed, unfortunately I cannot test all of them.

@ingDIY
Copy link
Contributor

ingDIY commented Jul 5, 2022

Ill follow this issue, because I can't route my OLED display to GPIO21-22!

@blazoncek
Copy link
Collaborator

The idea behind global allocation of I2C (or SPI in the future) would be something like:

  • get I2C pins from config (cfg.cpp)
  • allocate pins using PinManager (cfg.cpp; PinOwner::HW_I2C)
  • re-map/mux I2C interface to selected GPIO (cfg.cpp)

Then, in usermod, check like this:

  • if (getPinowner(usermod_gpio) == PinOwner::HW_I2C) then use HW API else allocate new pins and use SW API

Of course both of those would need appropriate change in Settings/Config which is more consuming task. Possibly also requiring changes elsewhere (set.cpp definitely and logic to change already allocated HW pins).

@ingDIY
Copy link
Contributor

ingDIY commented Jul 7, 2022

I tested the solution proposed by @softhack007: my display runs really faster now.
Solve this bug will be a good improvement!

@blazoncek
Copy link
Collaborator

blazoncek commented Jul 7, 2022

You can always just override HW_PIN_SDA (and SCL) so it will think those are HW pins.

@softhack007
Copy link
Collaborator Author

@blazoncek
Copy link
Collaborator

ATM yes.
But that is easy to fix. Isn't it? 😉

@softhack007
Copy link
Collaborator Author

softhack007 commented Jul 7, 2022

Yes agreed.
It might be good idea to have a "global override" that tells WLED at compile time what are the your primary I2C pins. Because they are usually shared (I2C is a bus, not a point-to-point interface).

#ifndef HW_PIN_SCL
   #define HW_PIN_SCL 22
#endif
#ifndef HW_PIN_SDA
   #define HW_PIN_SDA 21
#endif

and repeat for SPI pins

@blazoncek
Copy link
Collaborator

I have checked the Wire library and it looks like it uses HW accelerated I2C no matter which GPIOs are defined (except if they cannot be remapped by muxer).
That being said this is only true if Wire is uninitialised when usermod allocates/starts Wire. It turns out that dependency Time library has DS1307RTC class which calls Wire.begin() in its constructor. Unfortunately RTC is defined as a global variable which means the constructor is called prior to starting WLED routines and initialises Wire with platform default values (irrespective of WLED #defines) .
This was discovered by @Matoran (#2740)

@blazoncek
Copy link
Collaborator

Hopefully solved in #2737

softhack007 added a commit to atuline/WLED that referenced this issue Oct 20, 2022
- issue wled#2820 AEST Time Is Incorrect
- PR wled#2840 Add PKT time zone
- issue wled#2708 4LD unnecessary use of software I2C on ESP32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants