Skip to content

Display fix#122

Open
epi0n wants to merge 3 commits intofossasia:masterfrom
epi0n:fix-display-multiplexing
Open

Display fix#122
epi0n wants to merge 3 commits intofossasia:masterfrom
epi0n:fix-display-multiplexing

Conversation

@epi0n
Copy link

@epi0n epi0n commented Dec 30, 2025

Contrib shared with @michal-raczkowski - joint work during 39c3 :)

The fix turned out to be simple and consists of correcting pin numbering in the code so that the multiplexing logic works as expected.

20251230_193802.mp4

@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Dec 30, 2025

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adjusts the LED driver’s pin mapping for segment J to use the correct microcontroller pin so the display multiplexing works as intended.

Class diagram for updated LED pin mapping in leddrv

classDiagram
    class pindesc_t {
        char port
        int pin
    }

    class LEDDriver {
        +pindesc_t led_pins[LED_PINCOUNT]
    }

    LEDDriver "1" o-- "*" pindesc_t : uses

    class SegmentJMapping {
        +char port = B
        +int pin = 17
    }

    SegmentJMapping --|> pindesc_t : conforms_to
Loading

File-Level Changes

Change Details Files
Correct LED segment-to-pin mapping for segment J in the LED driver.
  • Update the pin descriptor for segment J to reference port B pin 17 instead of pin 15 in the led_pins array
  • Ensure the multiplexing logic uses the accurate hardware pin mapping for all display segments
src/leddrv.c

Possibly linked issues

  • #First leds of the first two columns are switched: PR fixes incorrect LED pin mapping, which directly addresses the swapped first-column LEDs described in the issue.

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Raczkowski,Michał and others added 2 commits December 30, 2025 19:41
@jerji
Copy link

jerji commented Jan 2, 2026

Implementing this pull request seems to mess up some lines on my USBC badge.

@epi0n
Copy link
Author

epi0n commented Jan 5, 2026

Implementing this pull request seems to mess up some lines on my USBC badge.

@jerji

Could you elaborate?

The lines are messed up as is with the current implementation. The fix does work with the badges that were handed out to us during 39C3 - as long as you provide USBC_VERSION=1 input parameter. Did you build the firmware with the USBC_VERSION argument set appropriately? :)

@mhummels
Copy link

mhummels commented Jan 6, 2026

Is it okay to use the USBC_VERSION Flag to distinguish these variants? In this other PR #117 the same Flag is used to distinguish between the USBC and MicroUSB Version. As far as I know, the currently sold version is already USBC but may use other Pins than the new hardware version

@epi0n
Copy link
Author

epi0n commented Jan 6, 2026

Is it okay to use the USBC_VERSION Flag to distinguish these variants? In this other PR #117 the same Flag is used to distinguish between the USBC and MicroUSB Version.

It's an educated guess to be honest, I added it in the following commit which kind of elaborates on the rationale:
Added defensive programming techniques in case the PIN definition was actually correct for a MicroUSB Board (which we don't have docs for)

It seems that the MicroUSB version was more popular back in the time, and the display did seem to work based on the video that is contained within the README.md file (link). Our assumption therefore was that perhaps the pin definition was different for MicroUSB boards, and it worked just fine there, but it's not the case for the USB-C version - hence why we added these defensive programming techniques (big words for an if condition)

If I had access to the MicroUSB PCB schema (or hardware in its' physical form) then I could confirm the above-mentioned with a higher degree of certainty.

As far as I know, the currently sold version is already USBC but may use other Pins than the new hardware version

I wasn't aware there are two USB-C versions of the board - I thought the original one was the one that was just a market-ready PCB, rather than a custom-tailored PCB (open hardware)?

@jerji
Copy link

jerji commented Jan 6, 2026

@epi0n Before applying your patch, my screen is fine, I have set the USBC_VERSION = 1 in the makefile and everything is great.

After applying your patch, two lines on my screens are switched it seems.

Maybe there are multiple usbc versions?

@mhummels
Copy link

mhummels commented Jan 12, 2026

I tried this patch on the new open hardware version https://github.com/fossasia/badgemagic-hardware/tree/main I got handed by the friendly people on the 39c3 ;-). It works perfectly fine when USBC_VERSION = 1 is set for building it.

But there are indeed 2 USBC Version PCBs out there. I bought another one (older USBC Version) on the 39c3. Please see them in comparison here:
Comparison of both USBC badgemagic

The upper one is the new generation (ng) variant which is the open hardware one found in the other github repository. The lower one is the one sold everywhere on the 39c3 with USBC, too. But this one still uses the old hardware pins for the LED matrix.

To sum up, I think there are actually 3 versions out in the field:

Variant Connector Build Flags Description
Rev1 MicroUSB None This is the first variant which I know of which was able to use the firmware without any build flags
Rev2 USBC USBC_VERSION = 1 Then there was a variant which got sold at least on 39c3 with USBC but still the old LED matrix pinout, without microphone and the two additional buttons KEY3 and KEY4
Rev3 / Open Hardware USBC USBC_VERSION = 1 NG = 1 This is the most recent one with KEY3 and KEY4 plus microphone and the new LED matrix pinout

I would sugget adding another build flag NG = 1 for this hardware variant and add it to the readme to avoid confusion. What do you think?

Maybe there is anybody at fossasia who knows more about the different hardware variants out there?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants