TM16xx is very unstable - shows strange characters most of the time

varadero commented 1 month ago

Describe the bug

I am using https://github.com/dotnet/iot/tree/main/src/devices/Tm16xx on Raspberry Pi 5 (.NET 8) to control 4 digit 7 segment display based on TM1637. Most of the times I get strange characters. Only about one out of 10 tries it is showing correct digits.

Steps to reproduce

        Tm1637 tm = new Tm1637(5, 6);
        tm.Brightness = 7;
        tm.ScreenOn = true;
        Character[] toDisplay = new Character[4] {
            Character.Digit1,
            Character.Digit2 | Character.Dot,
            Character.Digit3,
            Character.Digit4
        };
        tm.Display(toDisplay);
        Thread.Sleep(TimeSpan.FromSeconds(3));
        Character[] toDisplay2 = new Character[4] {
            Character.Digit9,
            Character.Digit8 | Character.Dot,
            Character.Digit7,
            Character.Digit6
        };
        tm.Display(toDisplay2);
        tm.Dispose();

Expected behavior

The display shows "12:34" followed by 1 second pause followed by "98:76"

Actual behavior

The displayed characters are very unstable. Sometimes when I start the application I can see "12:34" but then it turns to something like "...87..." (or simply the display does not show anything) where "..." is some unknown digit - looks like only part of the 7 segments of the digits are lit. If I dotnet build the app and start the application form the output folder multiple times, I get different results. Only about 1 out of 10 app starts will correctly display "12:34" followed by "98:76". Very often the "12:34" is also broken. If I restart the device (by switching its power off and on again) I eventually get "12:34" more often but "98:76" is still broken.

Versions used

dotnet --info - I am using VSCode and Remote SSH to develop directly on Raspberry Pi so dotnet is installed only on the Raspberry Pi 5 device:
```
Host:
Version:      8.0.2
Architecture: arm64
Commit:       1381d5ebd2
```

.NET SDKs installed: 8.0.201 [/home/pi/.dotnet/sdk]

.NET runtimes installed: Microsoft.AspNetCore.App 8.0.2 [/home/pi/.dotnet/shared/Microsoft.AspNetCore.App] Microsoft.NETCore.App 8.0.2 [/home/pi/.dotnet/shared/Microsoft.NETCore.App]

- Version of `System.Device.Gpio` package
```xml
<PackageReference Include="System.Device.Gpio" Version="3.1.0" />

Version of Iot.Device.Bindings package (not needed if bug is in System.Device.Gpio)
```
<PackageReference Include="Iot.Device.Bindings" Version="3.1.0" />
```

If I stop the dotnet app, don't change anything in the wiring and use this Python app - https://gitlab.com/matsievskiysv/gpiod-tm1637/-/tree/master/examples/clock (changed "/dev/gpiochip0" to "/dev/gpiochip4" because of Raspberry Pi 5) - I always get correct result no matter what - the display shows all the sample values specified in the python code without any glitch - tried it multiple times.

EDIT: The mentioned Python lib ( https://gitlab.com/matsievskiysv/gpiod-tm1637/-/blob/master/tm1637.py#L41 ) uses 10 microseconds delay between writes:

TM1637_DELAY = 10e-6  # 10us delay between clk/dio pulses

while Tm1637.cs uses 1 microsecond ( https://github.com/dotnet/iot/blob/8460898df2061230a5d45832a56a14d731d117e5/src/devices/Tm16xx/Tm1637.cs#L19C9-L19C17 ):

        // According to the doc, the clock pulse width minimum is 400 ns
        // And waiting time between clk up and down is 1 µs
        private const byte ClockWidthMicroseconds = 1;

Can we have ClockWidthMicroseconds configurable ?

EDIT2: I can confirm that setting the ClockWidthMicroseconds to 10 (I copy/pasted all the necessary .cs files inside https://github.com/dotnet/iot/tree/8460898df2061230a5d45832a56a14d731d117e5/src/devices/Tm16xx and also https://github.com/dotnet/iot/blob/8460898df2061230a5d45832a56a14d731d117e5/src/devices/Common/System/Device/DelayHelper.cs to my project and changed one line to private const byte ClockWidthMicroseconds = 10; ) made the code 100% working.

krwq commented 1 month ago

[Triage] @varadero are you interested in making a PR? We suspect the Pi 5 might faster than Pi 3/4 and timing issues could have showed up now

cc: @scegg

scegg commented 1 month ago

@varadero Your idea is good. We can edit Tm1637.cs, making sure that ClockWidthMicroseconds is set to 10. And a better way is make the ClockWidthMicroseconds as optional argument in all ctors with the default value is ClockWidthMicroseconds.

Make a property of ClockWidthMicroseconds may not be a good idea. Changing the delay while communicating with device may cause some error. And I guess we do not need to change that value after an instance created.

Your PR is very welcomed by me. Or, do you want me to make this PR instead?

jdbruner commented 1 month ago

I think that the proposed change is a good start. I played with the TM1637 recently on a Pico-W, and I found that it was pretty sensitive about timing. (I was using the PIO, so the timing was pretty accurate.) The datasheet doesn't mention it, but it looked to me like the TM1637 might have been doing some clock stretching on its sort-of I2C-like interface. At clock rates nearer what the datasheet claims the part supports, this narrowed the clock pulse for the ACK enough to cause problems.

varadero commented 1 month ago

@krwq I currently have other issues with the "customized" version of Tm1637.cs - "initially" (when starting it for the first time after turning the power of Raspberry Pi 5 on) it works but if I unplug the display 5V (provided from the Pi pin 2), plug it in again, the code stops working - the display doesn't show what it showed before unplugging the 5V power. If I now start the Python sample it somehow is able to "reset" the display and then I can use the customized Tm1637 version again. I need to figure out why this happens before I make PR.

varadero commented 1 month ago

@krwq I had to make non trivial changes in Tm16xxI2CLikeBase.cs as well in Tm1637.cs in order to fix all the issues I found. These changes include not only modifying clock width to 10 microseconds but also adding new and modifying existing Clock and Data pin signals and delays (aligning them to the Python code referenced above - https://gitlab.com/matsievskiysv/gpiod-tm1637/-/blob/master/tm1637.py ). I am afraid that my changes could not work on other device types (I am using Raspberry Pi 5) and I cannot test the changes on such devices. I could push my changes to a repository that people with different devices can clone and test on their devices before making PR for this repository.

Ellerbach commented 3 weeks ago

I could push my changes to a repository that people with different devices can clone and test on their devices before making PR for this repository.

[Triage] Yes, that's a great idea, please bring the code in a PR, even as a draft PR, so that we can make a call for test with different devices and validate that the fix works even if it takes a bit of time.

krwq commented 3 weeks ago

@varadero re-plugging issue sounds like a possible problem with reset sequence - btw when looking at other implementations (i.e. python) you should always look at the license first. In this case it's MIT so we're good but you should always check if compatible as we don't want any legal issues and make sure to give credits to the authors in the comment because it's the right thing to do regardless of license 😄 Also make sure to edit https://github.com/dotnet/iot/blob/main/THIRD-PARTY-NOTICES.TXT when copying or writing similar code

dotnet / iot

TM16xx is very unstable - shows strange characters most of the time #2312