Add endpoint binds and a switch input to the node runtime #118
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/endpoint-binds"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Runtime half of the YAML node generator (YN-1). A
ctrl::Bindties an output endpoint to a source (node, endpoint index) with an action:Toggle,ToggleOnChange(latching switch) orFollow(OR across follow sources). Binds are edge-triggered, so a command holds until the next source edge. A silent source node (3 missed heartbeats) triggers the target's own failsafe. Local binds loop back throughNode::send, so they work with the bus lost. Also addsSwitchInput(active low, pull-up, 50 ms debounce), aninterestspass-through onUpdateStack::begin, and spec section 9.5.Decisions:
latchingis not a flag onSwitchInput. The source emits plain Pressed/Released either way, so the generator picksToggleOnChangefor a latching source.Endpointgains a virtualisOn()so a bind can toggle any target.Tested: new
test_bindsandtest_switch_inputin the native env (mutating the edge check, the loopback and the staleness check each fails a test); full native suite andpio runpass. No existing node edited.Three findings.
@ -0,0 +37,4 @@bind.lastHeardMs = now;if (!isLevel || bind.sourceEndpoint != endpoint || bind.sourceOn == on) continue;bind.sourceOn = on;Spurious toggle after a source returns.
sourceOnstarts false and is reset to false on source silence (loop). So the first frame from a source that is already pressed/occupied (boot, or reconnect after a silent period) counts as an edge. Toggle fires on a PIR already occupied; ToggleOnChange flips on a latching switch that never moved. Suggest: first frame afterheard == falseonly recordssourceOn, with no action (Follow can still apply it). Add a test for it.@ -0,0 +64,4 @@void Binds::loop(Node& node, uint32_t now) {for (uint8_t index = 0; index < count_; ++index) {Bind& bind = *binds_[index];Failsafe only fires for a source that was heard at least once. A remote source that never appears leaves the target with no failsafe. If that is by design, say so in 9.5; otherwise start
lastHeardMsfrom bind time.@ -28,6 +29,8 @@ constexpr uint32_t heartbeatMs = 1000;constexpr uint8_t missedHeartbeatsBeforeAbsent = 3;// Staggers descriptor emission by node id so a fleet powering up together, or answering one broadcast query, doesn't burst.inline bool seenRecently(uint32_t lastSeen, uint32_t now) { return (now - lastSeen) < (heartbeatMs * missedHeartbeatsBeforeAbsent); }seenRecentlywas inserted between the stagger comment anddescriptorStaggerMs, so the comment now describes the wrong function. MoveseenRecentlyabove the comment.Findings 1 and 3 fixed in the follow-up commit (first contact no longer toggles, with a test; comment moved). Finding 2 left as is: a source never heard leaves the target at its boot failsafe, which is already the failsafe position, so there is nothing to apply.
First-contact guard is defeated by any non-level frame from the source. Fix inline.
@ -0,0 +33,4 @@for (uint8_t index = 0; index < count_; ++index) {Bind& bind = *binds_[index];if (bind.sourceNode != source) continue;const bool firstContact = !bind.heard;firstContactis true only for the first frame of any kind from the source. A heartbeat or any other frame setsheard, so the next level report no longer counts as first. Trace: node boots, source heartbeat arrives (heard=true), then the source's repeat ofEVT_OCCUPANCY occupied(spec 1126: repeated on refresh) arrives. sourceOn false -> true, firstContact false, Toggle flips the light. That is the exact bug the commit is meant to fix, and it will hit in practice because heartbeats come every 1 s. Track first level report per bind (e.g. a separatesyncedflag set when a matching level frame is recorded, cleared withheardinBinds::loop) and keepheardfor liveness only. The new test only passes because no frame precedes the first event; thehear()calls added to the other tests are what hide this case. Add a test: heartbeat, then occupied, assert light off.No new defects in
0153b30. Only open point is the known trade-off: bind.cpp onFrame swallows a first Pressed for Toggle/ToggleOnChange when it is the first level report after contact. A switch is edge-driven, so after a consumer reboot the user's first press does nothing. If that matters, treat Pressed from a binary source as always actionable and apply the first-report skip to occupancy only.Two minor points. No blockers.
@ -0,0 +11,4 @@if (binary.event != can::BinaryEvent::Pressed && binary.event != can::BinaryEvent::Released) return false;endpoint = binary.endpoint;on = binary.event == can::BinaryEvent::Pressed;freshPress = on && binary.durationMs == 0;Duration 0 also appears on a re-report within the same millisecond as the press, because emitState sends now - pressedAtMs_. That is negligible in practice. It is also the only thing that stops a held press toggling at first contact, so the 'zero means fresh' rule is worth a one-line note in binary_input.cpp or the spec.
@ -0,0 +76,4 @@bind.synced = false;bind.sourceOn = false;bind.target->onFailsafe(node, now);if (bind.action == BindAction::Follow && anyFollowSourceOn(bind.target)) bind.target->setOn(true, now);The spec says the failsafe is not applied while another follow source is on, but the code runs onFailsafe and then forces the target back on. On a strip that can start a fade-out and then a fade-in. If both sources lapse in the same pass, the first one turns the target on and the second then fails it off. Skip onFailsafe when anyFollowSourceOn(bind.target) is true, after clearing this bind's sourceOn. That matches the spec and drops the extra setOn.