Skip to content

Commit ad7c28a

Browse files
Apollon77claude
Apollon77
and
claude
authored
refactor(protocol,node): unify path logging via ExpandedPath/DataModelPath (#3664)
Routes Read/Write/Subscribe/Invoke diagnostics — both client request-formation and server-side response handlers — through a single DataModelPath produced by either ExpandedPath (wire paths) or resolvePathForSpecifier (specifier paths). Output now uses camelCase property names with hex fallback (`0.onOff.state.onOff`, `0.0xabcd.state.0x0`) instead of the dual `Name:0xHex` style. Markers `[ADD]` (listIndex===null) and `!` (isUrgent) attach to the previous segment without an intervening dot via a new "marker" segment type on DataModelPath. Fixes a wildcard-vs-concrete bug in Write request diagnostics: the formatter passed entry directly to resolvePathForSpecifier, which expects `attribute` (singular), while Write.Attribute carries `attributes` (plural). Concrete writes were therefore logged as `*` instead of the actual attribute name. inspectPath() now returns DataModelPath instead of string. Legacy callers using string interpolation/join keep working via toString(). Adds chunked-write detection: when chunked-list writes are emitted, the Write diagnostic includes an `chunked` flag alongside the path/value list. Removes obsolete toWildcardOrHexPath helper and node-local endpoint-name resolution (symmetric with client-side which has no endpoint names). Closes #2584 Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 5e5b095 commit ad7c28a

8 files changed

Lines changed: 249 additions & 115 deletions

File tree

‎packages/model/src/common/DataModelPath.ts‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@ import { Diagnostic } from "@matter/general";
1010
* Utility for tracking location in the Matter data model. This location is used for diagnostics.
1111
*
1212
* The path consists of a sequence of IDs, optionally with type information.
13+
*
14+
* Segments with type "marker" attach to the previous segment without an intervening "." separator (used for
15+
* diagnostic suffixes such as "[ADD]" or "!").
1316
*/
1417
export class DataModelPath implements Diagnostic {
1518
parent?: DataModelPath;
@@ -29,9 +32,10 @@ export class DataModelPath implements Diagnostic {
2932
return this.id;
3033
}
3134

32-
toString(includeType?: boolean) {
35+
toString(includeType?: boolean): string {
3336
if (this.parent) {
34-
return `${this.parent}.${this.#identity(includeType)}`;
37+
const sep = this.type === "marker" ? "" : ".";
38+
return `${this.parent.toString(includeType)}${sep}${this.#identity(includeType)}`;
3539
}
3640
return this.#identity(includeType).toString();
3741
}
@@ -45,13 +49,18 @@ export class DataModelPath implements Diagnostic {
4549

4650
get [Diagnostic.value](): Diagnostic {
4751
const result = Array<unknown>();
48-
for (const segment of this.toArray()) {
49-
if (result.length) {
52+
this.#appendDiagnostic(result);
53+
return Diagnostic.squash(result);
54+
}
55+
56+
#appendDiagnostic(result: unknown[]) {
57+
if (this.parent) {
58+
this.parent.#appendDiagnostic(result);
59+
if (this.type !== "marker") {
5060
result.push(".");
5161
}
52-
result.push(Diagnostic.strong(segment));
5362
}
54-
return Diagnostic.squash(result);
63+
result.push(Diagnostic.strong(this.id));
5564
}
5665

5766
at(id: string | number, type?: string): DataModelPath {

‎packages/node/src/node/integration/ProtocolService.ts‎

Lines changed: 45 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -49,15 +49,7 @@ import type {
4949
InteractionSession,
5050
NodeProtocol,
5151
} from "@matter/protocol";
52-
import {
53-
EventTypeProtocol,
54-
FabricManager,
55-
hasRemoteActor,
56-
Mark,
57-
OccurrenceManager,
58-
toWildcardOrHexPath,
59-
Val,
60-
} from "@matter/protocol";
52+
import { EventTypeProtocol, FabricManager, hasRemoteActor, Mark, OccurrenceManager, Val } from "@matter/protocol";
6153
import {
6254
AttributeId,
6355
AttributePath,
@@ -630,7 +622,7 @@ function invokeCommand(
630622
logger.info(
631623
"Invoke",
632624
Mark.INBOUND,
633-
Diagnostic.strong(`${path.toString()}.${command.name}`),
625+
Diagnostic.strong(path.at(camelize(command.name))),
634626
session.transaction.via,
635627
requestDiagnostic,
636628
);
@@ -723,7 +715,7 @@ function invokeCommand(
723715
logger.info(
724716
"Invoke",
725717
Mark.OUTBOUND,
726-
Diagnostic.strong(`${path.toString()}.${command.name}`),
718+
Diagnostic.strong(path.at(camelize(command.name))),
727719
session.transaction!.via,
728720
Diagnostic.dict(result),
729721
);
@@ -750,7 +742,7 @@ function invokeCommand(
750742
logger.info(
751743
"Invoke",
752744
Mark.OUTBOUND,
753-
Diagnostic.strong(`${path.toString()}.${command.name}`),
745+
Diagnostic.strong(path.at(camelize(command.name))),
754746
session.transaction.via,
755747
Diagnostic.dict(result),
756748
);
@@ -766,58 +758,56 @@ function invokeCommand(
766758
}
767759

768760
/**
769-
* Resolve a path into a human readable textual form for logging
770-
* TODO: Add a Diagnostic display formatter for this
761+
* Resolve a wire-format path into a {@link DataModelPath} for logging.
762+
*
763+
* Mirrors {@link ExpandedPath}'s segment shape (state./events./markers) but resolves cluster/element names against
764+
* the node-local protocol so custom clusters render with their type names rather than falling back to hex.
765+
*
766+
* Endpoint names from the node are intentionally not used — endpoints render by number — to keep client and server
767+
* logs symmetric.
771768
*/
772-
function resolvePathForNode(node: NodeProtocol, path: AttributePath | EventPath | CommandPath) {
769+
function resolvePathForNode(node: NodeProtocol, path: AttributePath | EventPath | CommandPath): DataModelPath {
773770
const { endpointId, clusterId } = path;
774-
const isUrgentString = "isUrgent" in path && path.isUrgent ? "!" : "";
775-
const listIndexString = "listIndex" in path && path.listIndex === null ? "[ADD]" : "";
776-
const postString = `${listIndexString}${isUrgentString}`;
777-
778-
const elementId =
779-
"attributeId" in path
780-
? path.attributeId
781-
: "eventId" in path
782-
? path.eventId
783-
: "commandId" in path
784-
? path.commandId
785-
: undefined;
786-
787-
if (endpointId === undefined) {
788-
return `*.${toWildcardOrHexPath("", clusterId)}.${toWildcardOrHexPath("", elementId)}${postString}`;
771+
const endpoint = endpointId !== undefined ? node[endpointId] : undefined;
772+
const cluster = endpoint && clusterId !== undefined ? endpoint[clusterId] : undefined;
773+
774+
let dmPath = new DataModelPath(endpointId ?? "*", "endpoint");
775+
dmPath = dmPath.at(nameOf(cluster?.type.name, clusterId), "cluster");
776+
777+
if ("attributeId" in path) {
778+
const attribute =
779+
cluster && path.attributeId !== undefined ? cluster.type.attributes[path.attributeId] : undefined;
780+
dmPath = dmPath.at("state").at(nameOf(attribute?.name, path.attributeId));
781+
if (path.listIndex === null) {
782+
dmPath = dmPath.at("[ADD]", "marker");
783+
}
784+
return dmPath;
789785
}
790786

791-
const endpoint = node[endpointId];
792-
if (endpoint === undefined) {
793-
return `${toWildcardOrHexPath("?", endpointId)}.${toWildcardOrHexPath("", clusterId)}.${toWildcardOrHexPath("", elementId)}${postString}`;
787+
if ("eventId" in path) {
788+
const event = cluster && path.eventId !== undefined ? cluster.type.events[path.eventId] : undefined;
789+
dmPath = dmPath.at("events").at(nameOf(event?.name, path.eventId));
790+
if ("isUrgent" in path && path.isUrgent) {
791+
dmPath = dmPath.at("!", "marker");
792+
}
793+
return dmPath;
794794
}
795-
const endpointName = toWildcardOrHexPath(endpoint.name, endpointId);
796795

797-
if (clusterId === undefined) {
798-
return `${endpointName}.*.${toWildcardOrHexPath("", elementId)}${postString}`;
796+
if ("commandId" in path) {
797+
const command = cluster && path.commandId !== undefined ? cluster.type.commands[path.commandId] : undefined;
798+
return dmPath.at(nameOf(command?.name, path.commandId));
799799
}
800800

801-
const cluster = endpoint[clusterId];
802-
if (cluster === undefined) {
803-
return `${endpointName}.${toWildcardOrHexPath("?", clusterId)}.${toWildcardOrHexPath("", elementId)}${postString}`;
804-
}
805-
const clusterName = toWildcardOrHexPath(cluster.type.name, clusterId);
801+
// Wildcard path with no element discriminator — render as bare element wildcard
802+
return dmPath.at("*", "element");
806803

807-
if (elementId !== undefined) {
808-
if ("eventId" in path) {
809-
const event = cluster.type.events[elementId];
810-
return `${endpointName}.${clusterName}.${toWildcardOrHexPath(event?.name ?? "?", elementId)}${postString}`;
811-
} else if ("attributeId" in path) {
812-
const attribute = cluster.type.attributes[elementId];
813-
return `${endpointName}.${clusterName}.${toWildcardOrHexPath(attribute?.name ?? "?", elementId)}${postString}`;
814-
} else if ("commandId" in path) {
815-
const command = cluster.type.commands[elementId];
816-
return `${endpointName}.${clusterName}.${toWildcardOrHexPath(command?.name ?? "?", elementId)}${postString}`;
817-
} else {
818-
throw new ImplementationError("Invalid path");
804+
function nameOf(name: string | undefined, id: number | undefined) {
805+
if (name !== undefined) {
806+
return camelize(name);
819807
}
820-
} else {
821-
return `${endpointName}.${clusterName}.*${postString}`;
808+
if (id === undefined) {
809+
return "*";
810+
}
811+
return `0x${id.toString(16)}`;
822812
}
823813
}

‎packages/node/test/endpoint/server/InteractionProtocolTest.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1777,7 +1777,7 @@ describe("InteractionProtocol", () => {
17771777
messenger,
17781778
interaction.BarelyMockedMessage,
17791779
),
1780-
).rejectedWith("Duplicate concrete command path RootNode:0x0.OnOff:0x6.on:0x1 on batch invoke");
1780+
).rejectedWith("Duplicate concrete command path 0.onOff.on on batch invoke");
17811781
});
17821782

17831783
it("handles StatusResponseError gracefully", async () => {

‎packages/protocol/src/action/protocols.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,9 @@ export interface NodeProtocol extends CollectionProtocol<EndpointProtocol> {
6969
attrsChanged: Observable<NodeProtocol.AttrChange, MaybePromise>;
7070

7171
/**
72-
* Inspects an Attribute- or Event path and log in human-readable form if possible
72+
* Resolve a wire-format path into a {@link DataModelPath} for human-readable diagnostics.
7373
*/
74-
inspectPath(path: AttributePath | EventPath | CommandPath): string;
74+
inspectPath(path: AttributePath | EventPath | CommandPath): DataModelPath;
7575
}
7676

7777
export namespace NodeProtocol {

‎packages/protocol/src/action/request/Specifier.ts‎

Lines changed: 42 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
* SPDX-License-Identifier: Apache-2.0
55
*/
66

7+
import { camelize } from "@matter/general";
8+
import { DataModelPath } from "@matter/model";
79
import type { ClusterId, CommandId } from "@matter/types";
810
import { ClusterType, EndpointNumber, Global, TlvSchema } from "@matter/types";
911
import { MalformedRequestError } from "./MalformedRequestError.js";
@@ -197,16 +199,12 @@ export namespace Specifier {
197199
}
198200
}
199201

200-
export function toWildcardOrHexPath(name: string, value: number | bigint | undefined) {
201-
if (value === undefined) {
202-
return "*";
203-
}
204-
return `${name ? `${name}:` : ""}0x${value.toString(16)}`;
205-
}
206-
207202
/**
208-
* Resolve a path into a human readable textual form for logging
209-
* TODO: Add a Diagnostic display formatter for this
203+
* Resolve a request-side specifier location into a {@link DataModelPath} for human-readable diagnostics.
204+
*
205+
* Mirrors the segment shape of {@link ExpandedPath} (state./events./markers) so client-side specifier paths and
206+
* server-side wire paths render consistently. Element names fall back to camelized hex IDs when the element is
207+
* unknown.
210208
*/
211209
export function resolvePathForSpecifier<const C extends Specifier.ClusterLike>(location: {
212210
endpoint?: Specifier.Endpoint;
@@ -216,42 +214,48 @@ export function resolvePathForSpecifier<const C extends Specifier.ClusterLike>(l
216214
command?: Specifier.Command<Specifier.ClusterFor<C>>;
217215
isUrgent?: boolean;
218216
listIndex?: number | null;
219-
}) {
217+
}): DataModelPath {
220218
const endpointId = Specifier.endpointIdOf(location);
221219
const cluster = location.cluster ? Specifier.clusterFor(location.cluster) : undefined;
222-
const attribute = location.attribute && cluster ? Specifier.attributeFor(cluster, location.attribute) : undefined;
223-
const event = location.event && cluster ? Specifier.eventFor(cluster, location.event) : undefined;
224-
const command = location.command && cluster ? Specifier.commandFor(cluster, location.command) : undefined;
225-
const isUrgentString = "isUrgent" in location && location.isUrgent ? "!" : "";
226-
const listIndexString = "listIndex" in location && location.listIndex === null ? "[ADD]" : "";
227-
const postString = `${listIndexString}${isUrgentString}`;
228-
229-
const clusterId = cluster?.id;
230-
const elementId = attribute ? attribute.id : event ? event.id : command ? command.id : undefined;
231-
232-
if (endpointId === undefined) {
233-
return `*.${toWildcardOrHexPath("", clusterId)}.${toWildcardOrHexPath("", elementId)}${postString}`;
220+
221+
let dmPath = new DataModelPath(endpointId ?? "*", "endpoint");
222+
dmPath = dmPath.at(cluster ? camelize(cluster.name) : "*", "cluster");
223+
224+
if (location.attribute !== undefined) {
225+
const name = nameForElement(location.attribute, cluster && Specifier.attributeFor(cluster, location.attribute));
226+
dmPath = dmPath.at("state").at(name);
227+
if (location.listIndex === null) {
228+
dmPath = dmPath.at("[ADD]", "marker");
229+
}
230+
return dmPath;
234231
}
235232

236-
const endpointName = toWildcardOrHexPath("", endpointId);
233+
if (location.event !== undefined) {
234+
const name = nameForElement(location.event, cluster && Specifier.eventFor(cluster, location.event));
235+
dmPath = dmPath.at("events").at(name);
236+
if (location.isUrgent) {
237+
dmPath = dmPath.at("!", "marker");
238+
}
239+
return dmPath;
240+
}
237241

238-
if (cluster === undefined || clusterId === undefined) {
239-
return `${endpointName}.*.${toWildcardOrHexPath("", elementId)}${postString}`;
242+
if (location.command !== undefined) {
243+
const name = nameForElement(location.command, cluster && Specifier.commandFor(cluster, location.command));
244+
return dmPath.at(name);
240245
}
241246

242-
const clusterName = toWildcardOrHexPath(cluster.name, clusterId);
243-
244-
if (elementId !== undefined) {
245-
if (event) {
246-
return `${endpointName}.${clusterName}.${toWildcardOrHexPath(typeof location.event === "string" ? location.event : "?", elementId)}${postString}`;
247-
} else if (attribute) {
248-
return `${endpointName}.${clusterName}.${toWildcardOrHexPath(typeof location.attribute === "string" ? location.attribute : "?", elementId)}${postString}`;
249-
} else if (command) {
250-
return `${endpointName}.${clusterName}.${toWildcardOrHexPath(typeof location.command === "string" ? location.command : "?", elementId)}${postString}`;
251-
} else {
252-
return "unknown";
247+
return dmPath.at("*", "element");
248+
249+
function nameForElement(specifier: string | object, resolved: { id?: number; name?: string } | undefined) {
250+
if (typeof specifier === "string") {
251+
return camelize(specifier);
252+
}
253+
if (resolved?.name !== undefined) {
254+
return camelize(resolved.name);
255+
}
256+
if (resolved?.id !== undefined) {
257+
return `0x${resolved.id.toString(16)}`;
253258
}
254-
} else {
255-
return `${endpointName}.${clusterName}.*${postString}`;
259+
return "?";
256260
}
257261
}

‎packages/protocol/src/action/request/Write.ts‎

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ export function Write(optionsOrData: Write.Options | Write.Attribute, ...data: W
6161
interactionModelRevision = Specification.INTERACTION_MODEL_REVISION,
6262
} = options;
6363

64+
let chunked = false;
65+
6466
const result = {
6567
timedRequest: !!timed || !!timeout,
6668
timeout,
@@ -69,15 +71,24 @@ export function Write(optionsOrData: Write.Options | Write.Attribute, ...data: W
6971
suppressResponse,
7072
interactionModelRevision,
7173

72-
[Diagnostic.value]: () =>
73-
Diagnostic.list(
74-
data.map(entry => {
75-
const { version, value } = entry;
76-
return `${resolvePathForSpecifier(entry)} = ${Diagnostic.json(
77-
value,
78-
)}${version !== undefined ? `(version=${version})` : ""}`;
79-
}),
80-
),
74+
[Diagnostic.value]: () => {
75+
const items = data.flatMap(entry => {
76+
const { version, value, endpoint, cluster, attributes } = entry;
77+
const valueString = Diagnostic.json(value);
78+
const list = Array.isArray(attributes) ? attributes : [attributes];
79+
const versionString = version !== undefined ? `(version=${version})` : "";
80+
return list.map(attribute =>
81+
Diagnostic.squash(
82+
Diagnostic.strong(resolvePathForSpecifier({ endpoint, cluster, attribute })),
83+
` = ${valueString}${versionString}`,
84+
),
85+
);
86+
});
87+
if (chunked) {
88+
return [Diagnostic.asFlags({ chunked: true }), Diagnostic.list(items)];
89+
}
90+
return Diagnostic.list(items);
91+
},
8192
} as Write;
8293

8394
for (const entry of data) {
@@ -132,6 +143,7 @@ export function Write(optionsOrData: Write.Options | Write.Attribute, ...data: W
132143
tlv instanceof ArraySchema &&
133144
!isAclOrExtensionPath({ clusterId, attributeId })
134145
) {
146+
chunked = true;
135147
writeRequests.push(
136148
...tlv
137149
.encodeAsChunkedArray(value, { forWriteInteraction: true })

0 commit comments

Comments
 (0)