duinoWitchery / hd44780

Extensible hd44780 LCD library
GNU General Public License v3.0
238 stars 58 forks source link

Arduino Uno R4 Issues/Status #41

Open mjs513 opened 1 year ago

mjs513 commented 1 year ago

As we talked about on the R4 forum here are some comments using the NOYITO 1602 16x2 LCD Module Shield https://www.amazon.com/gp/product/B07SYPPTWN/ref=ppx_yo_dt_b_search_asin_title?ie=UTF8&th=1.

From what I see its using a PCF8574 port expander, if that matters.

Results from running the I2CexpDiag:

********************************************************************
Serial Initialized
--------------------------------------------------------------------
I2CexpDiag - i2c LCD i/o expander backpack diagnostic tool
--------------------------------------------------------------------
hd44780 lib version: 1.3.2
--------------------------------------------------------------------
Reported Arduino Revision: 1.6.7
CPU ARCH: arm - F_CPU: 48000000
--------------------------------------------------------------------
SDA digital pin: 18 A4
SCL digital pin: 18 A4
--------------------------------------------------------------------
Checking for required external I2C pull-up on SDA - YES
Checking for required external I2C pull-up on SCL - YES
Checking for I2C pins shorted together - Shorted
********************************************************************
ERROR: SDA and SCL shorted together
********************************************************************
I2C bus not usable

Not sure why its showing SDA/SCL shorted.

HelloWorld, Serial2LCD, UpTime, CustomChars, ReadWrite, sketches are working with the display.

Ran the LCDProc sketch and got:

LCDProc : hd44780
9600 20x4

LCDiSpeed Test: Just showing jibberish on the display

LCDCharSet looks like its working

Thats not too bad.

bperrybap commented 1 year ago

First thing I noticed from the output is that SCL and SDA are defined to be the same pin. This can't be and indicates an issue somewhere.

While most sketches and libraries won't have an issue with this, as they are not using the pins directly, it is an issue for the diagnostic tool which needs the pin numbers as it directly tests the pins.

I found the issue. The issue is in the Arduino R4 platform core. For both variants somebody screwed up. Here is the code from the pins_arduino.h file

static const uint8_t SDA = WIRE_SDA_PIN;
static const uint8_t SCL = WIRE_SDA_PIN;

I'll bug this in the R4 core.

In terms of other sketches not working. All other sketches should be working unless there is some kind of issue.

The only sketch that looks like there is an issue (other than I2CexpDiag) is LCDiSpeed. It pushes things as hard and as fast as possible. If there is some kind of h/w of s/w timing issue, it will often show up in this sketch.

We need to get I2CexpDiag working to fully test the environment. To get it working you will need to patch a file in the R4 core. You will have to locate where the core is installed, then go down to renesas_uno/1.0.2/variants and patch the pins_arduino.h header files in MIMINA and UNOWIFIR4 directories to fix the issue.

You will want to change this:

static const uint8_t SDA = WIRE_SDA_PIN;
static const uint8_t SCL = WIRE_SDA_PIN;

to this

static const uint8_t SDA = WIRE_SDA_PIN;
static const uint8_t SCL = WIRE_SCL_PIN;

This will allow I2CexpDiag to work so it can test things. Once that works, we can start to look at what is happening with LCDiSpeed.

mjs513 commented 1 year ago

Made that change and also incorporated the PRs for SPI, Micros, and setClock while I was at it. Diag is now working:

********************************************************************
Serial Initialized
--------------------------------------------------------------------
I2CexpDiag - i2c LCD i/o expander backpack diagnostic tool
--------------------------------------------------------------------
hd44780 lib version: 1.3.2
--------------------------------------------------------------------
Reported Arduino Revision: 1.6.7
CPU ARCH: arm - F_CPU: 48000000
--------------------------------------------------------------------
SDA digital pin: 18 A4
SCL digital pin: 19 A5
--------------------------------------------------------------------
Checking for required external I2C pull-up on SDA - YES
Checking for required external I2C pull-up on SCL - YES
Checking for I2C pins shorted together - Not Shorted
--------------------------------------------------------------------
Scanning i2c bus for devices..
 i2c device found at address 0x27
Total I2C devices found: 1
--------------------------------------------------------------------
Scanning i2c bus for all lcd displays (4 max)
 LCD at address: 0x27 | config: P01245673H | R/W control: Yes
Total LCD devices found: 1
--------------------------------------------------------------------
LCD Display Memory Test
Display: 0
 Walking 1s data test:  PASSED
 Address line test: PASSED
--------------------------------------------------------------------
Each working display should have its backlight on
and be displaying its #, address, and config information
If all pixels are on, or no pixels are showing, but backlight is on, try adjusting contrast pot
If backlight is off, wait for next test
--------------------------------------------------------------------
Blinking backlight test: to verify BL level autodetection
If backlight is mostly off but
you briefly see "BL Off" on display with backlight on,
then the library autodetected incorrect BL level
and the library cannot autoconfigure the device
--------------------------------------------------------------------
Displaying 'uptime' on all displays
--------------------------------------------------------------------

With the LCDiSpeed see two screens before turning to gibberish: First screen shows ByteXfer: 1094us 2nd screen

16x2FPS:26,88ms
Ftime: 37.20ms

Running the LCDiSpeed400 = works better continually cycles through the screen data showed for the LCDiSpeed sketch

bperrybap commented 1 year ago

ByteXfer of 1094us is quite slow. It less than half that on something like a normal UNO or Mega. I'd say they have some Wire library issues.

What do you mean by "gibberish"? Does it lock up or is the screen garbage that keeps changing?

Interesting/odd that LCDiSpeed400 works better. Typically the faster clock has issues particularly since 400 KHz is overclocking the PCF8574. This would seem to point to a Wire library issue when using the default rate of 100Khz. But maybe there is an issue that just shows up more often at the lower clock rate? Hard to say without being able to probe the h/w with a logic analyzer and scope.

What is the ByteXfer time when using a 400 KHz clock? What happens when you run LCDlibTest ?

mjs513 commented 1 year ago

Ok lets do this one question at a time:

ByteXfer of 1094us is quite slow. It less than half that on something like a normal UNO or Mega. I'd say they have some Wire library issues.

yes there are know issues with I2C - there are a couple of issues open on the forum as well as against the core.

What do you mean by "gibberish"? Does it lock up or is the screen garbage that keeps changing?

Looks something like this: IMG-0900 and looks like it changes as different tests are run.

Interesting/odd that LCDiSpeed400 works better. Typically the faster clock has issues particularly since 400 KHz is overclocking the PCF8574.

Hooked up a LA at 400khz shows a clock of about 384khz. Upload is 1093us. About the same as at 100khz. At 100khz its running at 98khz

Ran LibTest400 and ran great everything display correctly including the wave pattern at end and the clock

With libtest at 100khz gets to printing the bytexfer and then cycles through the tests (blink works) but then the gibberish as I showed before or nothing displays.

bperrybap commented 1 year ago

When there is garbage like that, the host and the LCD have lost nibble sync. i.e. the LCD is creating a byte from one nibble of one byte and on nibble from the next byte. A nibble was either lost or a phantom extra nibble was sent.

The most common causes of this are from glitches on the I2C bus, or if LCD timing is violated. These type of issues can be difficult to track down.

Once nibble sync is lost it will never recover on its own. The only way to get back in sync is either by re-initializing the LCD to 4 bit mode - which includes a nibble sync sequence. Or a power cycle which then requires a an initialization to 4 bit mode.

For timing, the 400khz clock should transfer bytes faster than 100khz. Given it isn't, then they definitely have some strange issues in the Wire library. Like maybe the library is doing something stupid or it is crappy code that is creating some kind of added necessary latency.

Does your LA have i2c decoding? It would be interesting see a trace of the bus during the LCDiSpeed data writes to see what the transfers look like.

I'm going to have a look a the Wire code in that core to see if there is any obvious contributing to this.

bperrybap commented 1 year ago

Yep, Well that didn't take long to find what I believe to be the cause of why the write performance sucks. Took me like less than 2 minutes. This is in the code in Wire.cpp that Wire.endTransmission() calls, write_to(), to send the data to the slave

    while(err == FSP_SUCCESS && timeout_ms > 0 && bus_status == WIRE_STATUS_UNSET) {
      timeout_ms--;
      delay(1);
    }

That is a stupid way to try to handle a timeout. It is polling once per millisecond. It should tightly spin on the status flags and break out of the loop if timeout_ms milliseconds have elapsed. The code changes to correct this are very minor. IMO, the person who wrote this R4 Wire code is a moron and should not be writing code at this level as it is blatantly obvious this is not the proper way to write a loop that polls a status but has a timeout.

mjs513 commented 1 year ago

Good Morning Bill Yeah - there is either an issue (which I can not find) or it was posted in the forum. Will have to see if I can find it again. There is actually another spot where they use a delay(1).

Here is a whole discussion on it: https://forum.arduino.cc/t/bug-in-wire-timeout-loop-cause-poor-performance/1151584/3 and here https://github.com/arduino/ArduinoCore-renesas/issues/66

bperrybap commented 1 year ago

I saw those. Those demonstrate the same problem on the read side. Bottom line, the R4 Wire library seems to have lots of crap coding and is not function complete. Also the person who wrote that timeout loop in the Wire code shouldn't be coding at this level or at all.