LED indicator: led-bridge pulls lifecycle state from voice-command#14
Merged
Conversation
Julie Krasnick (jckras)
requested review from
Esha Maharishi (EshaMaharishi) and
Nicolas Palpacuer (NickPPC)
and removed request for
Esha Maharishi (EshaMaharishi)
May 21, 2026 18:57
Replace the push model (voice-command driving an led_indicator) with a pull model: - voice-command becomes a pure status source. It records its lifecycle state (idle/listening/thinking/responding) and exposes it via Status(); it has no knowledge of LEDs. Minimal footprint: one field + a setActivity helper, a "state" entry in Status(), and four one-line transition markers. - led-bridge becomes the puller. It depends on a configurable status_source, polls that resource's Status() on an interval, and writes the state to the serial LED firmware whenever it changes. All LED opinion lives here. This decouples the two: any resource exposing a "state" in Status() can drive the LEDs, and the firmware owns the visuals via a stable state-word contract over serial. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Julie Krasnick (jckras)
force-pushed
the
voice-command-led-indicator
branch
from
June 4, 2026 15:15
aee99bb to
299a7d9
Compare
Julie Krasnick (jckras)
changed the title
RDK's GetStatus serializes with raw structpb.NewStruct, which rejects
[]string list values. Status() returned commands as []string, so any
wire-level getStatus call (led-bridge polling, web-app, remote) would
error instead of returning the state. Convert commands to []interface{},
the same workaround beanjamin documents in its Status() methods.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Surface operational diagnostics so an operator can confirm from the dashboard whether the LED is actually wired up and receiving data: serial_port, baud_rate, messages_sent, bytes_sent, last_sent_at, and last_error/last_error_at. Counters are recorded on every write and guarded by a dedicated statsMu so a Status query never blocks behind a slow port.Write. Adapted from the diagnostics in the backup branch (PR #15), but exposed directly via Status()/getStatus rather than a DoCommand shim, since Status() is on the resource.Resource interface in rdk v0.122. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
- tick() committed lastState before the serial write, so a failed write was never retried — the LED stayed wrong until the next state change. Now commit lastState only after a successful write (or a deliberate ignore), so a transient serial hiccup self-heals on the next tick. - write() dedupes its failure log (warn only when the error changes) so a persistently broken port retried every tick doesn't flood the logs; Status() still surfaces the sticky last_error. - Reframe actionForState: forwarding the state word is the intended design (firmware owns the visuals), not a placeholder; collapse the four identical cases into one allow-list arm. - Remove references to a firmware/led-indicator/ dir and "firmware sketches in this repo" — the sketch is external, not in this repo. - Fix meta.json short_description: described the old DoCommand-forward model; led-bridge now polls status_source. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
No behavior change — tighten the comments added in this PR. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Julie Krasnick (jckras)
requested review from
Nicolas Palpacuer (NickPPC) and
Ale Paredes (ale7714)
June 4, 2026 16:06
Nicolas Palpacuer (NickPPC)
approved these changes
Jun 4, 2026
| logger: logger, | ||
| port: port, | ||
| source: source, | ||
| poll: poll, |
Collaborator
There was a problem hiding this comment.
maybe pollingPeriod ?
| b.logger.Warnw("status_source Status() failed", "err", err) | ||
| return | ||
| } | ||
| state, _ := status["state"].(string) |
Collaborator
There was a problem hiding this comment.
"state", the key to look at in the status response, should probably be a config value.
- Add a `state_key` config field (default "state") so led-bridge can read the lifecycle field from any status_source, not just sources that name it "state" — e.g. point it at a resource exposing "current_step" or "mode". Restores the "watch any source" decoupling the hardcoded key undercut. - Rename the `poll` struct field to `pollInterval` for clarity, matching the existing PollIntervalMs config field and defaultPollInterval const. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Julie Krasnick (jckras)
force-pushed
the
voice-command-led-indicator
branch
from
June 4, 2026 20:14
e4df6c9 to
b28f25a
Compare
5 tasks
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
3 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.
Summary
Wires up an LED indicator that mirrors voice-command's lifecycle, using an inverted (pull) dependency: the LED component depends on voice-command and reads its state via
getStatus, rather than voice-command knowing about LEDs.voice-commandis now a pure status source. It records its current lifecycle state and exposes it viaStatus()(thegetStatusRPC) under a"state"key. It has no knowledge of LEDs and never drives them. The four states:idle— wake-mode, waiting for the wake wordlistening— STT actively capturing (post wake-word, or in conversation follow-up)thinking— LLM interpret in flightresponding— speaking via TTSactivityStatefield + asetActivityhelper, a"state"entry inStatus(), and four one-line transition markers (ininterpret,speak, andlistenForCommand).viam:conversation-bundle:led-bridgeis the puller. It depends on a configurablestatus_source, polls that resource'sStatus()on an interval (default 200ms), reads"state", and writes the state to the USB-serial LED firmware only when it changes. All LED opinion lives inled-bridge(actionForState); the firmware owns the visuals via a stable state-word contract over serial.led-bridge reports serial-write health via its own
Status():serial_port,baud_rate,messages_sent,bytes_sent,last_sent_at, andlast_error/last_error_at. Lets an operator confirm from the dashboard whether the LED is actually wired up and receiving data ("0 sent, last_error: port not found" vs "412 sent, 2s ago"). Counters use a dedicated lock so a status query never blocks behind a slowport.Write.Config
{ "serial_port": "/dev/ttyUSB0", "status_source": "voice_command" }Optional:
state_key(default"state"),baud_rate(default 115200),poll_interval_ms(default 200).state_keyis the field led-bridge reads fromstatus_source'sStatus(). It defaults to"state"(what voice-command emits), but can point at any other key — e.g. a source exposing"current_step"or"mode"— so one led-bridge can drive lights for a completely different flow without code changes.Why pull instead of push
Decouples the two components. Any resource that exposes a
"state"inStatus()can drive the LEDs — point a secondled-bridgeat a differentstatus_sourcefor a different flow, with no changes to either component's internals. voice-command stays free of feature-specific LED logic, and a stuck serial port can never stall the voice loop (led-bridge polls on its own goroutine).Wire-serialization note
RDK's
GetStatusserializes with rawstructpb.NewStruct, which rejects[]string(and other typed slices) — it would error the entire call over the wire. voice-command'sStatus()now returnscommandsas[]interface{}accordingly. Rule of thumb for anyStatus()/DoCommandreturn: scalars,map[string]interface{}, and[]interface{}only; numbers come back asfloat64.Companion firmware
The ESP32 sketch (WS2812B strip) lives outside this repo and is flashed to the indicator hardware. It substring-matches the state word on each serial line to pick an animation. The current sketch handles
idle/listening/thinking; arespondingmode will be added firmware-side (led-bridge already forwards it). Any other indicator can be driven by a generic resource that consumes the same state-word protocol.Test plan
go build ./...andgo vetpassStatus()and led-bridgeStatus()maps verified structpb-serializable (wire-safe)led-bridgeopens the configured serial port on Reconfigure and logs port + baud + status_sourcevoice_command.getStatus()and writes to serial on state change onlyStatus()showsmessages_sent/last_sent_atadvancing,last_erroron a bad portlistening(blue pulse) →thinking(amber pulse) → back tolisteningon continued conversation →idle(dark) on conversation endrespondingforwarded over serial (lights once firmware adds the mode)