Skip to content

Conversation

heronet
Copy link
Contributor

@heronet heronet commented Sep 24, 2025

Add a new auxdisplay driver for TM1637 7-segment LED displays. The driver supports:

  • 4-digit 7-segment display output
  • Decimal point positioning
  • Brightness control (0-7 levels)
  • Display on/off control
  • Alphabet character support (A-Z, a-z) and digits (0-9)
  • Basic cursor positioning

The driver implements the standard AUXDISPLAY API

Copy link
Contributor

@kartben kartben left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you shouldn't be requiring a custom, device-specific, API for this - please implement this as an auxdisplay driver. Custom sample will likely also go away in the process

Thanks!

@heronet
Copy link
Contributor Author

heronet commented Sep 25, 2025

you shouldn't be requiring a custom, device-specific, API for this - please implement this as an auxdisplay driver. Custom sample will likely also go away in the process

Thanks!

Got it. I reimplemented it for the auxdisplay subsystem. Thanks

@heronet heronet requested a review from kartben September 25, 2025 10:24
@kartben kartben removed the area: LED Label to identify LED subsystem label Sep 26, 2025
@kartben kartben closed this Sep 26, 2025
@kartben kartben reopened this Sep 26, 2025
@thedjnK
Copy link
Contributor

thedjnK commented Sep 30, 2025

Also there is a lot of changing of GPIO mode from input to output, would it be better to just configure it at start-up as output then to input and output again in that function, and optionally in on/off functions?

@heronet
Copy link
Contributor Author

heronet commented Sep 30, 2025

Also there is a lot of changing of GPIO mode from input to output, would it be better to just configure it at start-up as output then to input and output again in that function, and optionally in on/off functions?

Yes. That is what I should have done in the first place

Edit: Well the CLK can be removed but the DIO pin does need to be reconfigured to OUTPUT before start/stop conditions because it's left in INPUT mode after reading the ACK signal in tm1637_send_byte()

Copy link
Contributor

@simonguinot simonguinot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @heronet,

Thanks for this PR.

Please find my review below !

@kartben kartben dismissed their stale review October 1, 2025 12:30

my initial comment has been addressed

@heronet heronet force-pushed the add-tm1637-driver branch from 8bb9dc4 to 3ef56cb Compare October 1, 2025 15:32
@zephyrbot zephyrbot added the area: Tests Issues related to a particular existing or missing test label Oct 1, 2025
@heronet heronet requested a review from simonguinot October 1, 2025 15:41
@heronet
Copy link
Contributor Author

heronet commented Oct 1, 2025

Hello @heronet,

Thanks for this PR.

Please find my review below !

Hi @simonguinot,
Thanks for reviewing it so thoroughly. I implemented your suggestions and I will be waiting for your review when you get the time.

@heronet
Copy link
Contributor Author

heronet commented Oct 5, 2025

Hi @thedjnK, @mbolivar, @kartben, @nashif, @xingrz, and other reviewers,

I wanted to check in on this PR for the TM1637 7-segment display driver. Please let me know if there are any remaining changes or feedback required to help move it forward. This is my first time sumbitting a driver so that would make sense.
Thank you for your time and reviews!

Add device tree binding for TitanMicro TM1637 7-segment LED
display controller. The TM1637 uses a GPIO bit-banging protocol
with clock and data pins.

Properties:
- clk-gpios: Clock GPIO pin
- dio-gpios: Data I/O GPIO pin
- bit-delay-us: Bit delay for GPIO protocol timing

Signed-off-by: Siratul Islam <[email protected]>
@heronet heronet force-pushed the add-tm1637-driver branch from 3ef56cb to b74ba5e Compare October 6, 2025 13:40
@heronet heronet requested a review from xingrz October 6, 2025 13:41
@heronet
Copy link
Contributor Author

heronet commented Oct 6, 2025

Hi @xingrz,
I believe I have addressed your review — could you please confirm or approve if all is okay?

Copy link
Member

@xingrz xingrz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good! Just a minor thought - it might be a bit clearer to define the segment codes using BIT(x) combinations instead of magic numbers. But it’s totally fine to keep it as is if you prefer.

@heronet
Copy link
Contributor Author

heronet commented Oct 6, 2025

Hi @xingrz,

Thanks for taking a look again! I kept them in hex since it follows the standard convention for 7-segment lookup tables (e.g., https://en.wikichip.org/wiki/seven-segment_display/representing_letters). The hex format is more compact and matches how most 7-segment drivers represent these patterns.

Happy to change to BIT() macros if you prefer though.

Thanks

@heronet heronet requested a review from xingrz October 6, 2025 15:24
xingrz
xingrz previously approved these changes Oct 6, 2025
@heronet heronet requested a review from mbolivar October 6, 2025 20:52
Add a new auxdisplay driver for TM1637 7-segment LED displays.

The driver supports:
- 4-digit 7-segment display output
- Decimal point positioning
- Brightness control (0-7 levels)
- Display on/off control
- Alphabet character support (A-Z, a-z) and digits (0-9)
- Basic cursor positioning

The driver implements the standard AUXDISPLAY API

Signed-off-by: Siratul Islam <[email protected]>
Add build test coverage for the TM1637 auxdisplay driver.

Creates gpio_devices.overlay for GPIO-based auxdisplay devices
and adds corresponding test case to verify the driver builds
correctly on native_sim platforms.

Signed-off-by: Siratul Islam <[email protected]>
@heronet heronet requested a review from xingrz October 6, 2025 21:25
Copy link

sonarqubecloud bot commented Oct 6, 2025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants