Fix CardputerADV being misdetected as plain Cardputer - #233
Merged
Conversation
The GPIO read used for the VAMeter / Cardputer / CardputerADV split was overwritten by the VAMeter I2C probe result, so the subsequent (result & 0x0C) == 0x0C test saw a bool and could never select CardputerADV. This surfaces whenever an I2C Cap or Unit is attached: on CardputerADV G5/G6 are the external pins, so the Cap pull-ups make (result & 3) == 3 true, the VAMeter probe runs, finds nothing and leaves result at 0. M5Unified then picks the plain Cardputer pin table, whose in_i2c entry is 255/255, and every access to a built-in device on the internal bus fails. Keep the probe result in its own variable so the G8/G9 bits survive.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
board_M5Cardputerwhenever an I2C Cap/Unit is attachedresult, so the G8/G9 bits of the GPIO read survive for the CardputerADV checkProblem
With any I2C Cap/Unit attached,
M5.getBoard()returnsboard_M5Cardputer(14) instead ofboard_M5CardputerADV(24). Plain Cardputer has no internal I2C, soM5Unified's pin table selectsin_i2c = 255, 255andM5.In_I2Ccomes up unusable:Every access to a built-in device on the internal bus then fails. It flips reliably with the Cap on
and off (reported with the M5Unit-NFC Cap on M5GFX 0.2.26 via M5Unified 0.2.19).
Root cause
In
src/M5GFX.cppthe variableresultis used for two different things and gets overwritten:Once the
(result & 3) == 3branch is taken,resultholds a bool, so(result & 0x0C) == 0x0Ccan never be satisfied and CardputerADV becomes undetectable. This holds regardless of how the read
bits are packed.
For reference,
lgfx::gpio::commandpacks MSB first (result = (result << 1) + resinsrc/lgfx/v1/platforms/esp32/common.cpp), reading G9, G8, G6, G5 in that order, giving bit0=G5,bit1=G6, bit2=G8, bit3=G9. So
(result & 3)tests G5/G6 (VAMeter's SYS I2C) and(result & 0x0C)tests G8/G9 (CardputerADV's SYS I2C) — both conditions match the pin table in the comment above
them. Only the reuse of
resultis wrong.Why a Cap triggers it
On CardputerADV, G5/G6 are the external (Cap/GROVE) pins. A Cap's I2C pull-ups hold them high, so
(result & 3) == 3becomes true, the VAMeter probe runs, finds nothing, and leavesresult == 0.Without a Cap those pins read low, the branch is skipped, and the CardputerADV check works on the
preserved GPIO value — which is why the bug only appears with something attached.
Why not reorder the checks
Evaluating the CardputerADV test first would also fix the symptom, but it introduces the opposite
misdetection: on VAMeter, G8/G9 are the external pins, so a VAMeter with an I2C Unit attached would
be detected as CardputerADV. The existing order (VAMeter first, CardputerADV behind the
board == board_M5Cardputerguard) is correct as designed, so this PR only separates the variablesand leaves the detection order untouched.
Known remaining case (not addressed here)
With a Cap attached to a CardputerADV, G5/G6/G8/G9 are all high and the VAMeter probe still runs on
G5/G6. A Cap that answers at both 0x40 and 0x41 would be detected as VAMeter. Closing that would
require probing a built-in device on the CardputerADV internal bus as well, which changes the
detection design rather than fixing the defect, so it is left out of this PR.
Validation
currently have an I2C Cap on hand to confirm the fix on a CardputerADV. Happy to hold this PR
until that is confirmed if you prefer.