-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Programmatically-readable color led deck self-test results, add I2C address pin test #1572
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Test I2C_ADDRESS pin on Color LED deck connectivity by toggling it LOW/HIGH and verifying the deck MCU reads the correct state via I2C command. Ensures proper signal routing during production testing.
…led test results Enhanced the production self-test for Color LED decks with programmatically-readable test results and integrating LED position verification inside deck self test: - Added deckTest param group with bitmap test results for both decks (bit 0: protocol version, bit 1: LED position, bit 2: I2C address pin) - Replaced pollLedPosition() with verifyLedPosition() that checks LED is mounted on correct PCB side (bottom/top) and fails test on mismatch - Added LED position constants matching deck firmware values (NONE=0x00, BOTTOM=0x01, TOP=0x02) - Removed LED position from log groups (now verified during self test only) - Added comprehensive debug output for all test failures This enables automated production testing to detect hardware assembly errors like LEDs mounted on wrong side.
| uint8_t actualPosition = response[1]; | ||
| ctx->ledPosition = actualPosition; | ||
|
|
||
| const char* expectedStr = (expectedPosition == COLORLED_LED_POS_BOTTOM) ? "BOTTOM" : "TOP"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be only used for debug printing, what about putting it inside an #ifdef DEBUG and inside the if so it's only used if the comparison fails?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is important info to print, but we only need to (and do) in case of failure, so I moved constructing the string inside the failure case
evoggy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Enhanced the production self-test for Color LED decks with programmatically-readable test results and integrating LED position verification inside deck self test:
Requires: