Color-correction (gamma) read-back - #230
Merged
Merged
Conversation
Today the app can SET color correction (set_color_correction, ack'd only
via welcome) but there is no read-back, so the Color Correction page can't
show the device's current gamma and a gamma HITL journey can only assert the
device ack'd -- not that it applied the value it was set to.
Add a get_color_correction request + color_correction_state reply, mirroring
the get_hardware_config / hardware_config_state pattern:
- Proto: GetColorCorrection {} and ColorCorrectionState (resolved per-channel
gamma_r/g/b + lum_r/g/b). The device always has a concrete profile (WS2812B
after a reboot), so the state carries resolved values, never a profile name.
Regenerated the checked-in TS bindings (web/src/gen); micropb + protobuf-es
bindings are build-time.
- Firmware: CMsg::GetColorCorrection -> color_correction_state(), reporting
whatever the last set_color_correction stored (the shared session core, so
esp32c6 and esp32c3 both get the arm -- both build clean). Updated the two
exhaustive matches in phone_client_frames.rs.
- Web: client.getColorCorrection() (mirrors getHardwareConfig) + the Color
Correction page now hydrates its curves from the device on open (adopting
the device's reported values as the clean baseline, not a dirty edit) and
falls back to the locally-remembered profile on older firmware. Wired the
driver harness too.
- Tests: set->get round-trip unit test in session.rs (echoes gamma +
luminance, incl. a partial gamma-only override) mirroring
hardware_config_roundtrip, and a getColorCorrection wire round-trip in
web/tests/client.test.ts.
Journey hook (not implemented here -- the pi/hitl/phone journey files belong
to the iOS agent / #227, and gamma_config lands after #227): the future
gamma_config journey can now setColorCorrection({gamma}) then
getColorCorrection() and assert color_correction_state echoes the set gamma
("device applied gamma = X"), instead of only checking the welcome ack. Mock
device support for the new get was NOT added: mock_device.py lives under
pi/hitl/phone/ (owned by #227), so it is left to that agent to avoid a
collision.
Verified: bazel test //firmware/player:session_test
//firmware/player:phone_client_frames_test //web:web_ts_typecheck_test
//web:unit_tests //web:client_test (green); bazel build -c opt
//firmware/player_app:esp32c6_netstack and :esp32c3_netstack (ok).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
This branch was successfully deployed
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
2 participants
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.
Color-correction (gamma) read-back
Today the app can set color correction (
set_color_correction, ack'd only viawelcome) but there is no read-back, so the Color Correction page can't show the device's current gamma, and a gamma HITL journey can only assert the device ack'd — not that it applied the value it was set to.This adds a
get_color_correctionrequest +color_correction_statereply, mirroring the existingget_hardware_config/hardware_config_statepattern.Protocol (
shared/protocol/proto/ledmapper.proto)GetColorCorrection {}— client request (get_color_correction = 35).ColorCorrectionState— server reply (color_correction_state = 20): resolved per-channelgamma_r/g/b+lum_r/g/b. The device always has a concrete profile (the WS2812B default after a reboot), so the state carries resolved values, never a profile name.web/src/gen/ledmapper_pb.ts) via buf/protobuf-es; micropb (firmware) and Python bindings are generated at build time.Firmware (
firmware/player/src/lib.rs)CMsg::GetColorCorrection→color_correction_state(), reporting whatever the lastset_color_correctionstored (the same(gamma, luminance)the LUTs are built from).esp32c6andesp32c3get the arm for free — both build clean (-c opt). No C++/board change needed (the reply serializes through the generatedServerMessageencoder likehardware_config_state).firmware/player/tests/phone_client_frames.rs(reply_allowed+arm_name).Web
client.getColorCorrection()(web/src/net/client.ts) mirrorsgetHardwareConfig.web/src/ui/screens/colorCorrection.ts) now hydrates its curves from the device on open — adopting the device's reported gamma/luminance as the clean baseline (not a dirty edit, so nothing is pushed back and the save button stays idle), and falling back to the locally-remembered profile on older firmware without the read-back. Stale "no device read-back" comments updated.web/src/net/proto.ts; driver harness case added inweb/src/driver/harness.ts.Tests
color_correction_roundtripinfirmware/player/tests/session.rs:set_color_correction→get_color_correctionechoes the gamma + luminance (incl. a partial gamma-only override falling back to the WS2812B luminance), mirroringhardware_config_roundtrip.getColorCorrection round-trips the device's applied gammainweb/tests/client.test.ts(wire round-trip; float32-exact values since the proto fields arefloat).Journey hook (not implemented here)
The
pi/hitl/phone/journey files belong to the iOS agent / #227, and thegamma_configjourney lands after #227. With this read-back in place, that future journey can:i.e. assert
color_correction_stateechoes the gamma it was set to, instead of only checking thewelcomeack.Mock device:
mock_device.pysupport for the newgetwas not added —pi/hitl/phone/mock_device.pysits under the #227-owned directory, so it's left to that agent to avoid a collision. (The Pi pydantic serverhandler.pyalready returnsunknown_typefor unimplemented arms, same as it does forset_color_correctiontoday.)Verification
bazel test //firmware/player:session_test //firmware/player:phone_client_frames_test //web:web_ts_typecheck_test //web:unit_tests //web:client_test— green.bazel build -c opt //firmware/player_app:esp32c6_netstackand:esp32c3_netstack— ok.Notes for reviewers
client.tsand the proto are also touched by the concurrent Diffuse-fixture LED capture: strided blink + adaptive blob detector #152 work (hardwareSetup/behavior-settings/capture); these additions are additive and staged narrowly — expect a rebase.🤖 Generated with Claude Code