feat(sbmd): seed event-driven resource values from attributes#211
feat(sbmd): seed event-driven resource values from attributes#211rchowdcmcsa wants to merge 2 commits intomainfrom
Conversation
rchowdcmcsa
requested review from
cleithner-comcast,
kfundecmcsa,
mkkoch and
tleacmcsa
as code owners
There was a problem hiding this comment.
Pull request overview
Adds “seedFrom” support so SBMD event-driven resources can be initialized from the attribute cache at configure/synchronize time, and formalizes event-mapper suppression semantics (script may return {} to skip publishing without error). This improves initial state availability for event-only resources like door locks.
Changes:
- Introduces
mapper.seedFromin SBMD schema/spec/model + parser validation and ID stamping. - Seeds initial resource values from the Matter attribute cache during device configure and synchronize.
- Updates door-lock spec + virtual device behavior, and adds/updates unit + integration tests and documentation.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| testing/test/door_lock_test.py | Adds integration tests for initial seeding and subsequent event-driven updates. |
| testing/mocks/devices/matterjs/src/DoorLockDevice.js | Emits LockOperation events on sideband lock/unlock to drive event-mapped updates. |
| openspec/specs/sbmd-system/spec.md | Documents event suppress contract and introduces seedFrom requirement. |
| openspec/specs/matterjs-door-lock-device/spec.md | Specifies sideband ops must emit LockOperation events. |
| openspec/changes/sbmd-event-resource-seeding/tasks.md | Tracks implementation tasks/checklist for the change. |
| openspec/changes/sbmd-event-resource-seeding/specs/sbmd-system/spec.md | Delta spec documenting added/modified SBMD requirements for this change. |
| openspec/changes/sbmd-event-resource-seeding/specs/sbmd-seed-from-attribute/spec.md | Detailed change spec for seedFrom semantics/constraints. |
| openspec/changes/sbmd-event-resource-seeding/specs/matterjs-door-lock-device/spec.md | Delta spec for MatterJS door lock event emission requirement. |
| openspec/changes/sbmd-event-resource-seeding/proposal.md | Proposal narrative: motivation, impact, and high-level design. |
| openspec/changes/sbmd-event-resource-seeding/design.md | Design doc describing runtime architecture and trade-offs. |
| openspec/changes/sbmd-event-resource-seeding/.openspec.yaml | Registers the openspec change entry and creation date. |
| docs/SBMD.md | Documents seedFrom mapper and event suppression behavior. |
| core/test/src/sbmdParserTest.cpp | Updates door-lock parser assertions and adds seedFrom parser tests. |
| core/test/src/SbmdScriptTest.cpp | Adds unit test for seedFrom-style attribute read mapping (uint8 → boolean). |
| core/deviceDrivers/matter/sbmd/specs/door-lock.sbmd | Moves locked to event-driven mapping + seedFrom from LockState. |
| core/deviceDrivers/matter/sbmd/sbmd-spec-schema.json | Adds seedFromMapper schema and mapper.seedFrom property. |
| core/deviceDrivers/matter/sbmd/mquickjs/SbmdScriptImpl.h | Updates script output extraction documentation (and precondition). |
| core/deviceDrivers/matter/sbmd/mquickjs/SbmdScriptImpl.cpp | Implements event suppression handling and return-type guarding in MapEvent. |
| core/deviceDrivers/matter/sbmd/SpecBasedMatterDeviceDriver.h | Adds configure/synchronize overrides and seeding helper declaration. |
| core/deviceDrivers/matter/sbmd/SpecBasedMatterDeviceDriver.cpp | Binds seedFrom info, registers seedFrom mappers, seeds values at configure/sync. |
| core/deviceDrivers/matter/sbmd/SbmdSpec.h | Extends mapper model with initial-read (seedFrom) fields. |
| core/deviceDrivers/matter/sbmd/SbmdScript.h | Documents MapEvent suppression semantics and caller responsibilities. |
| core/deviceDrivers/matter/sbmd/SbmdParser.cpp | Parses seedFrom and enforces cross-field validation (event required, read excluded). |
| core/deviceDrivers/matter/MatterDevice.h | Adds seedFrom binding + seeding API and storage for seedFrom bindings. |
| core/deviceDrivers/matter/MatterDevice.cpp | Implements seedFrom binding/seeding and treats suppressed events as no-op. |
| * If the script omits the "output" key but returns a plain object (e.g., returns {}), | ||
| * the event is intentionally suppressed: MapEvent returns true with outValue left empty. | ||
| * The caller MUST check outValue.empty() and skip updateResource in that case. | ||
| * This is useful when an event type carries multiple operation sub-types, only some of | ||
| * which represent a resource state change. For example, a LockOperation event may carry | ||
| * a lock, unlock, or door-sense operation; a script can return {} for sub-types it does | ||
| * not need to propagate, avoiding spurious resource updates. |
There was a problem hiding this comment.
The MapEvent contract now requires callers to treat outValue.empty() as "suppressed". This makes it impossible for an event mapper to publish an intentionally empty string value (since empty becomes suppression). Consider adjusting the interface to represent suppression explicitly (separate flag/optional/enum) rather than overloading the output string's emptiness.
There was a problem hiding this comment.
There is no scenario (so far) where the empty-string case is a valid resource value in Barton. No event mapper would ever have a reason to publish an empty string as a legitimate value. You are technically correct in your concern, but practically speaking I don't think the churn for closing this gap is worth addressing at this time.
Event-driven resources (those using mapper.event) previously had
no initial value until the first event fired. This change adds a
seedFrom mapper type that reads the corresponding attribute from
the device's attribute cache at configure and synchronize time,
giving the resource an immediate value without waiting for an event.
As part of this work, the event mapper's suppress contract was formally
defined and correctly implemented — scripts may return {} to intentionally
skip an update (e.g., for non-state-change event sub-types), which is now
handled as a clean no-op rather than an error.
Key design decisions:
- seedFrom is a companion to mapper.event, not a replacement for mapper.read;
read+seedFrom is rejected by the parser as a configuration error
- seedFrom reuses the mapper.read script interface (sbmdReadArgs context
variable) so the same script shape applies to both
- SeedFrom bindings are stored in a separate resourceSeedFromBindings map and
are never added to readableAttributeLookup, preventing seedFrom attributes
from being treated as subscribable reads
- MapEvent now has a tri-state contract: false = script error; true + empty
outValue = suppress; true + non-empty outValue = publish. A non-object
return (null, undefined, primitive) is always a script error, guarded by an
object type check that matches the MapWrite/MapExecute pattern.
Implementation:
- SbmdSpec: added hasInitialRead / initialReadAttribute / initialReadScript
to SbmdMapper
- SbmdParser: parse seedFrom block; reject event alias, missing script,
read+seedFrom conflict, and seedFrom-without-event; stamp resource IDs onto
initialReadAttribute in SetMapperIds
- sbmd-spec-schema.json: added seedFromMapper definition and seedFrom as
optional mapper property
- MatterDevice: added BindResourceSeedFromInfo and SeedResourceFromAttribute;
the latter reads from the device attribute cache, runs MapAttributeRead,
and calls updateResource
- SpecBasedMatterDeviceDriver: added DoConfigureDevice and DoSynchronizeDevice
overrides that call SeedInitialResourceValues, which iterates all resources
with hasInitialRead and invokes SeedResourceFromAttribute for each non-skipped
resource
- SbmdScriptImpl (MapEvent): added object type guard before suppress check;
suppress path clears outValue and returns true rather than calling
ExtractScriptOutputAsString
- door-lock.sbmd: locked resource changed from mapper.read to mapper.event
(LockOperation, 0x0002) + mapper.seedFrom (LockState attribute, 0x0000);
event script correctly uses sbmdEventArgs; lockOperation added as second
prerequisite
- DoorLockDevice.js: handleLock and handleUnlock now emit LockOperation events
inside the act() callback in addition to updating lockState
Tests:
- sbmdParserTest: updated test_sbmdParserDoorLockFile to assert read=false and
hasInitialRead=true; added 5 new seedFrom tests covering valid spec,
seedFrom-without-event, missing script, read+seedFrom conflict, and
event-alias-rejected
- SbmdScriptTest: added MapAttributeReadUint8ToBoolean covering locked (1),
unlocked (2), and not-fully-locked (0) states against the door-lock seed
script
- door_lock_test.py: added test_locked_resource_seeded_on_commission and
test_locked_resource_updated_by_event integration tests
Refs: BARTON-355
rchowdcmcsa
force-pushed
the
rchowd/dev/eventResourceInitialState
branch
from
ace7157 to
c72e8ed
Compare
| JSValue outputVal = JS_GetPropertyStr(ctx, outJson, "output"); | ||
|
|
||
| if (JS_IsException(outputVal)) | ||
| { | ||
| icError("Exception reading 'output' field for cluster 0x%X, event 0x%X: %s", | ||
| eventInfo.clusterId, | ||
| eventInfo.eventId, | ||
| GetExceptionString(ctx).c_str()); | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| if (JS_IsUndefined(outputVal)) | ||
| { | ||
| icDebug("Event mapper returned no output (suppressed) for cluster 0x%X, event 0x%X", | ||
| eventInfo.clusterId, | ||
| eventInfo.eventId); | ||
| outValue.clear(); | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| return ExtractScriptOutputAsString(outJson, outValue); |
There was a problem hiding this comment.
MapEvent() reads the script result’s "output" property to detect the suppress case, but if it’s present the code then calls ExtractScriptOutputAsString(outJson, ...) which re-reads "output" again. This duplicates property access/work and can be avoided by refactoring to extract/convert the already-fetched outputVal (or adding a helper that both checks presence and converts).
There was a problem hiding this comment.
Refactoring ExtractScriptOutputAsString() to convert the already-fetched outputVal introduces the potential for user error imo (every call to ExtractScriptOutputAsString() would have to satisfy a precondition that the outputVal was already fetched from outJson and validated -- and that's a lot of code duplication in its own right, unless the fetch/validation is made into its own helper, but then you'd just have to call two helpers everywhere).
MapEvent() pre-fetches "output" to distinguish suppress from publish before any conversion happens. Since we already have the value in hand, I'm going to change MapEvent() to convert it directly in-line to avoid the redundant property lookup that calling ExtractScriptOutputAsString() would introduce. The code duplication introduced in this one spot by doing so is an acceptable trade-off in my mind. The helper remains appropriate for the other mappers where no pre-fetch is needed.
Event-driven resources (those using mapper.event) previously had no initial value until the first event fired. This change adds a seedFrom mapper type that reads the corresponding attribute from the device's attribute cache at configure and synchronize time, giving the resource an immediate value without waiting for an event.
As part of this work, the event mapper's suppress contract was formally defined and correctly implemented — scripts may return {} to intentionally skip an update (e.g., for non-state-change event sub-types), which is now handled as a clean no-op rather than an error.
Key design decisions:
Implementation:
Tests:
Refs: BARTON-355