Repository navigation
Test and expand Matter bridge mappings and device sharing feedback - #28
Draft
jeroenwienk wants to merge 10 commits into
Draft
jeroenwienk wants to merge 10 commits into
jeroenwienk wants to merge 10 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Auto-mode setpoint synchronization and failure-safe cleanup have unresolved correctness and reliability issues.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds comprehensive Matter bridge mapping contracts, backend comparison tooling, and fixes mapping/lifecycle behavior.
Changes:
- Corrects power, lighting, thermostat, lock, cover, and sensor mappings.
- Adds 38 synthetic mapping variants with behavioral, inventory, lifecycle, and infrastructure tests.
- Adds optional pinned Homey OS backend comparison and reporting.
File summaries
| File | Description |
|---|---|
.gitignore |
Ignores generated mapping reports. |
README.md |
Documents mapping tests. |
package.json |
Adds mapping test scripts. |
standalone.mjs |
Uses the named server export. |
lib/MatterBridgeApp.mjs |
Uses the named server export. |
lib/MatterBridgeServer.mjs |
Fixes mappings and lifecycle handling. |
lib/runInMicrotask.mjs |
Adds deferred callback utility. |
tests/bridge-devices.test.mjs |
Updates server import. |
tests/device-updates.test.mjs |
Updates server import. |
tests/mapping-behavior.test.mjs |
Adds end-to-end mapping contracts. |
tests/mapping-infrastructure.test.mjs |
Tests RPC and cleanup infrastructure. |
tests/mapping-inventory.test.mjs |
Validates capability inventory. |
tests/mappings/BackendComparison.mjs |
Compares bridge behavior with backend. |
tests/mappings/BridgeHarness.mjs |
Provides Matter controller harness. |
tests/mappings/BridgeTransport.mjs |
Implements backend transport boundary. |
tests/mappings/README.md |
Documents mapping infrastructure. |
tests/mappings/Rpc.mjs |
Implements IPC request handling. |
tests/mappings/SimulatedDevice.mjs |
Simulates Homey devices. |
tests/mappings/backend-allowances.json |
Records narrow backend differences. |
tests/mappings/backend-inventory.json |
Classifies backend capabilities. |
tests/mappings/backend-worker.mjs |
Runs backend adapter logic. |
tests/mappings/backend.mjs |
Orchestrates backend comparison. |
tests/mappings/coverage.mjs |
Validates and reports coverage. |
tests/mappings/eventually.mjs |
Adds asynchronous polling helper. |
tests/mappings/manifest.mjs |
Defines mapping fixtures and expectations. |
tests/mappings/matchesAttributePath.mjs |
Matches subscription paths. |
tests/mappings/report.mjs |
Generates mapping reports. |
tests/mappings/selectMappings.mjs |
Supports focused fixture selection. |
Review details
Suppressed comments (1)
lib/MatterBridgeServer.mjs:1519
- The same cleanup short-circuit affects device disable/delete: if any capability instance rejects during destruction, the endpoint is never deleted, but
disableDevice()has already removed the ID fromenabledDeviceIdsand swallows this error. Re-enabling can then add a duplicate endpoint over stale state. Continue cleanup after individual failures and only report an aggregate once the endpoint and maps have been reconciled.
for (const instance of Object.values(this.deviceCapabilityInstances[device.id] ?? {})) {
await instance.destroy();
}
delete this.deviceCapabilityInstances[device.id];
await deviceEndpoint.delete();
- Files reviewed: 27/28 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+164
to
173
| for (const instances of Object.values(this.deviceCapabilityInstances)) { | ||
| for (const instance of Object.values(instances)) { | ||
| await instance.destroy(); | ||
| } | ||
| } | ||
| this.deviceCapabilityInstances = {}; | ||
| if (this.serverNode) { | ||
| await this.serverNode.close(); | ||
| } | ||
| this.serverNode = null; |
Comment on lines
+795
to
+799
| if (hasHeat && request.mode !== Thermostat.SetpointRaiseLowerMode.Cool) { | ||
| writes.set('target_temperature', this.state.occupiedHeatingSetpoint / 100); | ||
| } | ||
| }, | ||
| ), { | ||
| id: 'main', | ||
| thermostat: { | ||
| systemMode: (() => { | ||
| switch (device.capabilitiesObj?.thermostat_mode?.value) { | ||
| case 'off': return Thermostat.SystemMode.Off; | ||
| case 'auto': return Thermostat.SystemMode.Auto; | ||
| case 'cool': return Thermostat.SystemMode.Cool; | ||
| case 'heat': return Thermostat.SystemMode.Heat; | ||
| default: return Thermostat.SystemMode.Heat; | ||
| if (hasCool && request.mode !== Thermostat.SetpointRaiseLowerMode.Heat) { | ||
| writes.set(coolingCapability, this.state.occupiedCoolingSetpoint / 100); |
jeroenwienk
changed the title
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
Bridge mappings could advertise incorrect initial values, retain stale values after restart, or accept controller commands without applying their complete effect to Homey. Correct power scaling, lock state, thermostat modes and fractional setpoints, light modes and transitions, cover direction and unsupported operations, sensor readings, and source availability. Refresh legacy color readings before cluster validation so persisted values outside the current temperature range cannot prevent the light endpoint from starting. Synchronize source device names while retaining existing pairings and endpoint identities.
Add water alarms, official fan percentage control, imported/exported cumulative energy, battery reporting, PM1, voltage/current, and independent channels for supported standard sub-capabilities. Show supported and omitted capabilities in device selection and fix the nested settings scrollbars. Custom app capabilities remain excluded; verified first-party Homey OS extensions are eligible but their missing mappings remain explicit.
Forward brightness and color move/step/stop commands and scene recall to Homey. Propagate source rejections and cancel managed transitions on Stop, failure, and device teardown. Omit optional timed unlocking; reject cover operations absent from the source. Identify explicitly advertises no physical effect. The command audit now links the formerly unverified commands to independent behavioral tests and rejects missing or failing evidence.
Validation
bafec2787531c8065e49622a73ed6a42f23b3669, with documented narrow allowances and no unexplained mismatches. The three affected temperature/color light variants were rerun after the legacy-state initialization fix.--skip-buildis unsuitable for this workflow.Release limits and follow-up
Keep this PR in draft pending the release decision and final Google Home UI confirmation. Live inventories and detailed reports stay private. Physical coverage applies only to the tested device and command path; other actuator families still need coordinated hardware checks where available.
Native Homey fades use source duration options. Stop holds the currently reported source value, so its physical accuracy depends on the source driver's feedback. Scene recall applies the sceneable fields exposed by the current Matter model; ordinary hue storage and EnhancedHue are not added. Overall air-quality ratings remain unknown when only numeric measurements exist. Unknown boolean readings use reachability rather than inventing a false value.
tests/mappings/CONTROLLER-ISSUES.mdretains the Home Assistant endpoint-event ordering issue and the Matter.js client structure race isolated by the harness. OnOff Lighting and air-conditioner feature-conformance follow-ups remain documented. The three verified Homey OS extensions remain missing mappings; no custom capability semantics are inferred.Public repository hygiene
Fixtures and upgrade storage are synthetic. Real installation data, device names and identifiers, network details, credentials, and live logs stay outside the repository. Generated reports and build output are ignored. The backend checkout is unchanged and is not required by public Node 22 CI.