STM32 I2C two wire LCD - bug fix and soft I2C driver implementation

[Bob-the-Kuhn]

This PR does two things:

• Fixes a bug in u8g_dev_ssd1306_sh1106_128x64_I2C.cpp that results in a dead display.
• Adds a soft I2C driver so that any pins can be used as SDA and SCL.

The bug in u8g_dev_ssd1306_sh1106_128x64_I2C.cpp results in extra bytes being sent during the init phase which results in a dead display. All the bytes in the init sequence were being processed by u8g_WriteEscSeqP_2_wire which injects the instruction byte/command for every entry. Turns out only the very first entry needed to have the instruction byte added. The solution was rename the init sequences and create a new routine (u8g_Write_Init_Sequence_2_wire) to process them.

The com driver u8g_com_stm32duino_ssd_i2c.cpp was added to support a soft I2C implementation. A problem with this implementation is that the only place to declare the flag that enables the soft I2C is in the compile time environment. Defining LCD_I2C_SOFT in configuration.h or in the pins_YOUR_BOARD.h file does not work.

In order to use the soft I2c you need to do the following:

1. copy the current environment & give it a unique name
2. add the following lines to the new environment

lib_deps                    = stevemarple/SoftWire@^2.0.9
                              stevemarple/AsyncDelay@^1.1.2
build_flags                 = -DLCD_I2C_SOFT

3. add the new environment to the corresponding line in pins.h.
4. compile using the new environment

To enable the use of u8g_com_stm32duino_ssd_i2c, the following files were modified: marlinui_DOGM.h, marlinui_DOGM.cpp & HAL_LCD_com_defines.h.

The flag ALTERNATIVE_LCD now selects between SPI and I2C interfaces for the SSD1306 and SH1106 LCDs. This flag is in configuration.h.

TESTING

The changes have been tested on SSD1306 and SH1106 displays attached to STM32F4 and STM32F7 boards.

OPEN ITEMS

Testing on STM32F1 motherboard. I've ordered new units but it'll be about 2 weeks before I get them from China.
Testing on LPC1768 motherboard to be sure that still works.

[Bob-the-Kuhn]

The routines are hard wired to use DOGLCD_SDA and DOGLCD_SCL as the pin names.

Just using SDA and SCL triggers some existing EEPROM logic which results in the hard wired I2C being in slave mode rather than master mode. Rather than mess with something that works I just grabbed the pin names from the pins file for the only controller that currently has I2C enabled (pins_GODI_CONTROLLER_V1_0.h).

[thisiskeithb]

Related Issue:

• [BUG] SSD1306 I2C display does not work #26389

Related Pull Request:

• move i2c pin setup for U8GLIB_I2C_OLED #26409

[Bob-the-Kuhn]

I have not been able to reproduce the failing tests. Would appreciate a pointer to the test docs so I can try to mimic the settings & reproduce the issues.

[Bob-the-Kuhn]

Looks like I need to investigate further.

pr #26409 has a very different solution. If it truly fixes the problem then maybe this PR should be killed.

I think it's best to use different pin names for SDA and SCL depending on the application. That would allow multiple I2C devices on a single board.

Where should the I2C setup be handled? Right now it's in both marlin_ui.cpp and u8g_dev_ssd1306_sh1106_128x64_I2C.cpp. I think it should only be in u8g_dev_ssd1306_sh1106_128x64_I2C.cpp.

[ellensp]

pr #26409 only addressed using alternative hardware i2c pins, and was tested on a Creality v4.2.2 and a 0.96" 128X64 SSD1306 LCD

See Original issue #26389

[Bob-the-Kuhn]

Just added a commit that does the following:

1. Added protection to keep from un-necessarily compiling u8g_dev_ssd1306_sh1106_128x64_I2C.cpp and u8g_com_stm32duino_ssd_i2c.cpp

2. Removed not needed I2C init from marlinui.cpp. Need to check non-STM32 implementations.

[dbuezas]

I can confirm this works on STM32H723VG (BigTreeTech SKR3-EZ board).
I was using u8g2 instead for software I2C in this branch of mine and had lost all custom icons.

By the way, the i2c clock at 100k leads to a very slow screen. At least in my case (SSD1309_128X64 in an ulticontroller) the screen works way better at 400k. I even went ahead and modified the softwire lib to take a delayMicroseconds of zero (I commented them out, no delay at all) and it works even better.

[dbuezas]

I have an RGB led controller on the same i2c bus as the LCD, is there any way to access the soft wire instance from out of the u8g_com_stm32duino_ssd_i2c.cpp file?.

Sorry for the tangent and thanks for your work :)

[dbuezas]

I measured the clock speed generated by SoftWire with the maximum frequency: 138kHz. Removing the delayMicroseconds from the library it goes up to 205kHz.
I think this is just because it uses digitalWrite and pinMode which are slow. I was hoping more out of a 500MHz mcu, even when using the arduino framework to control gpio.

[dbuezas]

This PR + MarlinFirmware/U8glib-HAL#31 + removing delayMicroseconds (SoftWire) + using platform specific gpio manipulation (SoftWire) gives amazing results. Maybe faster than stock HW i2c

[dbuezas]

Have you considered maxing out clock speed by default?
The maximum is 500_000 but results in "only" 138kHz in an SKR3 (550MHz).

[Bob-the-Kuhn]

Done but it didn't seem to affect anything. I've set both to 400K but both actuals come in at about 100K. I expected to get more speed on the hardware driver version. I'll definitely investigate.

[dbuezas]

I made the experiment of removing all delayMicrosecond calls from the SoftWire library and that made it reach something like 200kHZ, then I replaced all pin operations with platform specific ones and it reached 260kHz.
There is another bunch of optimisations which have a very big impact here.

[jmz52]

The com driver u8g_com_stm32duino_ssd_i2c.cpp was added to support a soft I2C implementation. A problem with this implementation is that the only place to declare the flag that enables the soft I2C is in the compile time environment. Defining LCD_I2C_SOFT in configuration.h or in the pins_YOUR_BOARD.h file does not work.

pinmap_peripheral() can be used to check if provided pins support hardware I2C.
Touch screen code uses it to switch between hardware and software SPI in runtime.
xpt2046.cpp#L48

[Bob-the-Kuhn]

I was able to use pinmap_peripheral to create a version that made the hard/soft decision at run time.

The downside is that both the hard and soft libraries have to be included in the image so the size is always larger than with a compile time decision. The code is also uglier/larger because I had to split the code into almost identical hard and soft branches. I tried to get around this by using a system of pointers to the class functions. Unfortunately I couldn't find a way to compile and then execute them. I think this was because the hard and soft classes were not derivatives of a common class so the compiler couldn't figure out how to link to execute using the pointers.

I switched from the SoftWire soft I2C library to the SlowSoftWire library because SoftWire kept causing hard bus faults. That's strange because it was stable before I made these changes.

This has been tested on a couple of STM32F407 boards. Much more testing to come.

OPEN ITEM:
The SlowSoftWire library doesn't automatically install. The github repository is missing a library.json file

[jmz52]

In the long run we should aim to only include hardware/software code that is needed. A wrapper that always presumes the presence of both will end up being slower and will end up including unused codepaths that bloat the size of the binary.

I've checked the original issue and it looks like there is no bug in HW I2C code.
Creality V4.2.2 board has not I2C EEPROM, so no one really cared about it's I2C configuration until #26389

The [env:STM32F103RE_creality] uses generic MARLIN_F103Rx board_build.variant, with I2C pins defined in variant.h as

#define PIN_WIRE_SDA            PB7
#define PIN_WIRE_SCL            PB6

These are global parameters for arduinoststm32 framework and they can't be overridden from Marlin pins.h file.
This limitation is the reason we have multiple boards variants for same MCU. There is no "one size fit all" variant.
If needed, there global parameters can be overwritten by build_flags and build_unflags options in platormIO ini file.

So the solution for the original issue would be to either create proper variant for creality board or to force required values via build_flags.

This implementation with user-defined pins for HW I2C can lead to situation where we have two HW I2C controllers used to connect various devices - one using pins specified in variant.h and another one using pins DOGLCD pins.
Second I2C will only be used by 12864 screen and no other devices connected to it will work, as the default Wire class still uses pins defined in variant.h.
And it also lead to increased firmware size as both HW and SW libraries are linked.

There is a way to prevent firmware bloating if we make these settings a bit more restrictive:

• HW I2C pin are defined in variant.h or build_flags in ini file (no changes here)
• optional DOGM I2C pins are defined in pins.h
• #if DOGM are defined and are different from HW I2C pins - SW I2C is used, HW I2C is linked only of there are other I2C devices
• #else - HW I2C is used, SW I2C is not linked (current behavior)

[thinkyhead]

@jmz52 — Any effort to get our pins very well defined and under control will help a lot going forward. I keep pointing to my LCD Pins Refactor PR as an exemplar of getting more peripherals under control along with lots of documentation so each LCD file contains helpful information. I don't know how much that informs the exposure of SPI, Serial, and I2C pins for each board, but we should definitely provide hints someplace accessible — each board's pins file, most likely.

It would be great to get each distinct MCU defined in some consistent manner and then build specific board definitions on top of that. For STM32 devices the "boards" JSON files and "variants" folders provide some of both of those layers, both defining pins for the MCU and mapping them to their specific low-level functions, while our pins files take care of Marlin-specific mappings of those pins. Our pins files ought to contain more info about the available hardware ports for SPI, I2C, and Serial so we don't have to dig around in "variants."

I'm not sure where to begin with all that, but it's the kind of thing I will be thinking about a lot as I go forward with that LCD Pins Refactor PR.

[thinkyhead]

So the solution for the original issue would be to either create proper variant for creality board or to force required values via build_flags.

@jmz52 — I think it's better to have a separate variant if the overridden things would be the i2c pins PIN_WIRE_SDA and PIN_WIRE_SCL (and probably others). Most of the time the variants don't wrap these in #ifndef but we could also do that if it seems neater, or if only a small change is needed. I presume that most (all?) of the time the pins in variants.h correspond closely with the PeripheralPins.cpp pin function assignments.

Anyway, what do we want to set the i2c pins to, and which boards currently need the changes?

There is a way to prevent firmware bloating if we make these settings a bit more restrictive…

All very good suggestions, and in line with what I hope we're aiming for. If LCD pins correspond to HW pins then we select HW, and if they aren't then we select SW, and this is transparent to the user.

My current emphasis for the next release is to get bugs fixed and pins sorted out, so it would be good to get variants fixed up and add commentary to the code to guide us towards improvements in the allocation of i2c and SPI ports. The best I can do with this is to fix it up and get it to pass CI … but is this PR still useful, and how important is it to the long-term?

[dbuezas]

Will this ever be considered to be merged?
I have been using it since the PR was created and keep rebasing it and whatnot and it makes my own PRs hard to handle :)