Skip to content

Follow cross-references in device type tables - #90

Merged
AryaHassanli merged 14 commits into
project-chip:mainfrom
hasty:granbery/name_references
Oct 8, 2026
Merged

AryaHassanli merged 14 commits into
project-chip:mainfrom
hasty:granbery/name_references

Conversation

@hasty

@hasty hasty commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

This unifies the general parsing of entity ID & Name pairs in device types to allow for references instead of names. It also fixes the association of anchors within table rows to the entities defined in those rows (e.g. Conditions).

It adds several new error types to deal with mismatches between reference targets and the IDs and names in those tables.

@hasty
hasty requested a review from a team as a code owner October 7, 2026 08:15
@mergify

mergify Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

This pull request does not currently match the merge queue conditions, so it cannot be queued from here. The box comes back if it matches again.

@AryaHassanli AryaHassanli left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this — unifying the ID/Name pairs into EntityReference is a nice simplification, and having anchors inside table rows resolve to the row's entity is a welcome fix.

I built base (c2fc3ce) and head (c39f994), ran go test ./... (all pass) and staticcheck, then ran alchemy dm --force -a in-progress from both builds on current spec master and diffed the output and logs. This turned up a few regressions the tests don't catch:

Blocking (seen in the A/B run)

  1. Tag requirements: 16 new fatal errors, e.g. BatteryStorage.adoc:248 - mismatched namespace name in tag requirement: 0x0F vs. Power Source (also HeatPump, ElectricalEnergyTariff, ElectricalDistributionEnclosure). TableColumnNamespaceID is passed as a name column, and each error is reported twice.
  2. Dropped header variants:
    • Cluster requirements no longer read Cluster Name, so the templated RM device types (HepaFilter, ActivatedCarbonFilter, WaterTankLevel) now generate <cluster id="{RM_CL_ID}" name="" ...>.
    • Element requirements now read only Cluster Name, but the spec header is Cluster, so element-requirement cluster names and xrefs are lost completely.

Likely bugs
3. ElementRequirement.Clone() no longer copies Element (used for composed device types in composition.go).
4. ElementRequirement.Equals no longer compares Element.
5. Possible nil-pointer panic in EntityReference.Equals / ElementReference.Equals when only one side has an XRef (typed-nil passed into CrossReference.Equals).
6. Three swallowed errors in requirements.go (staticcheck SA4006 on lines 38, 255, 279).
7. dm composed path now passes deviceType instead of dt to renderClusterRequirement.
8. DeviceTypeReferenceTypeMismatch is built with a nil DeviceType and with the target entity as Source, because entity is shadowed.

Smaller points are in the inline comments. Once fixed, a dm A/B on the current spec should show zero diff apart from any intended changes. It would also be great to add tests for xref resolution, the new mismatch errors, and anchor-in-table-row lookup.

Comment thread matter/spec/requirements.go Outdated
Comment thread matter/spec/namespace.go Outdated
Comment thread matter/spec/requirements.go Outdated
Comment thread matter/spec/requirements.go Outdated
Comment thread matter/spec/requirements.go
Comment thread matter/spec/devicetype.go
Comment thread matter/spec/devicetype.go
Comment thread matter/spec/devicetype.go
Comment thread matter/spec/devicetype.go
Comment thread asciidoc/table.go
@AryaHassanli

Copy link
Copy Markdown
Contributor

Thank you @hasty for addressing the comments.
Could you please take a look at the Build error? There seems to be an issue.

@hasty

hasty commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

Thank you @hasty for addressing the comments. Could you please take a look at the Build error? There seems to be an issue.

Yeah, auto-complete changed it on me without me noticing. Fixed.

@AryaHassanli
AryaHassanli merged commit 241cd68 into project-chip:main Oct 8, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants