feat(http): expose probe device model via Devices accessor#185
Draft
ottobolyos wants to merge 6 commits into
Draft
feat(http): expose probe device model via Devices accessor#185ottobolyos wants to merge 6 commits into
ottobolyos wants to merge 6 commits into
Conversation
ottobolyos
added a commit
to ottobolyos/mtconnect.net
that referenced
this pull request
Jun 1, 2026
73e9c11 to
e99fde5
Compare
ottobolyos
added a commit
to ottobolyos/mtconnect.net
that referenced
this pull request
Jun 1, 2026
ottobolyos
added a commit
to ottobolyos/mtconnect.net
that referenced
this pull request
Jun 2, 2026
The doc-comments commit on MTConnect-Compliance-Tests shifted the line of the FixtureDirEnv read in CppAgentParityWorkflowTests.cs from 488 to 499. The generated page embeds source line numbers, so the change desynced docs/reference/environment-variables.md and the in-sync test in MTConnect.NET-Docs-Tests failed on every integration branch layered on top of this PR (TrakHound#185, TrakHound#186).
ottobolyos
added a commit
to ottobolyos/mtconnect.net
that referenced
this pull request
Jun 2, 2026
ottobolyos
added a commit
to ottobolyos/mtconnect.net
that referenced
this pull request
Jun 2, 2026
ottobolyos
added a commit
to ottobolyos/mtconnect.net
that referenced
this pull request
Jun 2, 2026
ottobolyos
added a commit
to ottobolyos/mtconnect.net
that referenced
this pull request
Jun 2, 2026
ottobolyos
added a commit
to ottobolyos/mtconnect.net
that referenced
this pull request
Jun 2, 2026
ottobolyos
added a commit
to ottobolyos/mtconnect.net
that referenced
this pull request
Jun 2, 2026
ottobolyos
added a commit
to ottobolyos/mtconnect.net
that referenced
this pull request
Jun 2, 2026
The doc-comments commit on MTConnect-Compliance-Tests shifted the line of the FixtureDirEnv read in CppAgentParityWorkflowTests.cs from 488 to 499. The generated page embeds source line numbers, so the change desynced docs/reference/environment-variables.md and the in-sync test in MTConnect.NET-Docs-Tests failed on every integration branch layered on top of this PR (TrakHound#185, TrakHound#186).
Adds a new HttpClientDeviceModel fixture that drives the full MTConnectHttpClient against AgentRunner's embedded server, exercising both surfaces issue TrakHound#176 calls out before they are fixed: - the public Devices snapshot accessor (currently missing — compile- error RED), with separate tests for the pre-probe empty state, the post-probe populated state and snapshot-not-live-view semantics; - the DeviceReceived event (currently raised zero times because ProcessProbeDocument iterates an empty list — behavioural RED), with one test pinning the per-device fire count on the first probe and another pinning the IDataItem Container / Device back-pointers the issue calls out as the wired model's selling point. Both invariants will go green when the matching MTConnectHttpClient changes land.
Adds a public read-only Devices snapshot accessor on MTConnectHttpClient and fixes DeviceReceived to actually fire for every parsed device. The client already cached the wired device model in a private _devices dictionary populated by ProcessProbeDocument, but never exposed it; the only public path to the post-probe model was ProbeReceived, which carries the raw IDevicesResponseDocument before InstanceId stamping. The new accessor returns an independent Dictionary copy under the same internal lock the populate path uses, so callers can enumerate without synchronising against the worker thread. DeviceReceived previously built an empty outputDevices list, populated only _devices in the first loop, then iterated the empty list — so the event was raised zero times across the lifetime of every client. The fix folds the invocation into the populate loop, keeping the cache and the event in lockstep and dropping the dead second loop. The new public Devices member shadows the MTConnect.Devices namespace inside the class scope, so the two pre-existing in-class references to that namespace are now fully qualified as MTConnect.Devices.* to keep their resolution unambiguous. The matching HttpClientDeviceModel test fixture, added in the previous commit, transitions from RED to GREEN: the missing accessor was a compile-error RED, the never-fired event was a behavioural RED, and both invariants now hold end to end against the AgentRunner-backed embedded HTTP server. Closes TrakHound#176.
Tighten the Devices property XML doc to state that the returned dictionary is a fresh allocation independent of the cache while the IDevice values are shared references replaced wholesale on each probe (F-001, F-003, F-006). Fix BrE "synchronising" → "synchronizing" (F-002). Add remarks to DeviceReceived noting that handlers fire in document order while the cache is still being populated (F-007). Add disambiguation comments at the two MTConnect.Devices fully-qualified sites where the Devices property shadows the namespace (F-004). Trim the 5-line historical comment in ProcessProbeDocument to a single forward-looking line (F-005).
Adds /// doc comments to the HttpClientDeviceModel test fixture and each of its five [Test] methods so the project survives the campaign's 100% XML-doc coverage gate (CS1591 → error). The original block comment on the class is promoted to a /// summary; each test gains a one-line summary that pins the behaviour the test name expresses. The gap surfaced when this PR's branch was union-merged into the integration branch on top of feat/docs-full-coverage (TrakHound#182), which turns CS1591 into a build error for every test project.
8213389 to
c5ff7b7
Compare
Lands four §19 review findings on the issue-176 surface: - F-001: clear the _devices cache at the start of every probe inside the same lock block that clears _cachedComponents and _cachedDataItems so devices the agent no longer advertises are evicted before the new probe is loaded. The Devices accessor's XML doc claimed the snapshot reflected the most recent probe; without the clear, stale UUIDs persisted indefinitely. - F-002: wrap DeviceReceived?.Invoke in try/catch and route subscriber exceptions through InternalError. A throwing handler previously aborted the populate loop mid-stream, leaving the cache half-filled and suppressing ProbeReceived for that probe. The Worker loop's outer catch around the same fan-out at line 860 already used this pattern; this finding extends it to the per-device event. - F-004: wrap the snapshot in ReadOnlyDictionary so a consumer cannot mutate the cache through a downcast. The Devices XML doc promised a read-only contract; without the wrapper the runtime type stayed a plain Dictionary that downcasts could mutate. - F-005: tighten spaced em-dashes in the DeviceReceived summary to the unspaced form per CONVENTIONS §1.0d-decies. Adds three NUnit tests pinning the new invariants and closing two §10a negative-coverage gaps the review surfaced (F-008, F-009): - DeviceReceivedFiresOnEverySubsequentProbe — restarts the client so a second probe round-trip runs through the same agent and asserts the event re-fires for every device on the second probe. - DevicesAccessorReflectsLatestProbeWithStaleEntriesEvicted — pins the F-001 eviction contract. - DeviceReceivedHandlerThrowingDoesNotBreakCachePopulation — pins the F-002 isolation contract.
… pages Closes the §19 docs-completeness invariant gap CONVENTIONS §1.0d-vicies- quinquies names — every new public surface ships with matching narrative docs in the same PR. The Devices accessor and the now-firing DeviceReceived event landed in feat/issue-176 with XML doc comments only; the auto-generated docfx API page picks those up, but the consumer-facing example and configure pages did not mention either surface. Adds: - docs/examples/client-http.md — a 'Subscribing to the wired device model' subsection under the document-client example, showing a DeviceReceived handler walking the wired DataItems and a Devices snapshot read after the first probe completes. Cites issue TrakHound#176 and the 2026-06-01 approval. - docs/configure/consumer.md — a paragraph in the .NET-HTTP-client subsection naming Devices and DeviceReceived alongside the existing CurrentReceived guidance, plus a note that throwing handlers route through InternalError without breaking the cache fill. Resolves F-006 + F-007 in the §19 ledger at extra-files.user/plans/23-issue-176/review-findings.md.
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
Closes #176. Adds a public
Devicesread-only accessor onMTConnectHttpClientthat returns a lock-guarded snapshot of the post-probe device tree, and fixesDeviceReceivedto fire for every parsed device. The previous implementation built an emptyoutputDeviceslist, populated_devicesonly, then iterated the empty list, so the event was raised zero times across the lifetime of every client. Together the two changes give consumers two clean paths to the post-processed device model: event-driven viaDeviceReceived, snapshot-on-demand viaDevices.libraries/MTConnect.NET-HTTP/Clients/MTConnectHttpClient.cs: addpublic IReadOnlyDictionary<string, IDevice> Devicesreturning anew Dictionary<string, IDevice>(_devices)under_lock; restructureProcessProbeDocumentto raiseDeviceReceivedinside the same loop that populates_devices, dropping the deadoutputDeviceslist and the never-fired second loop. The newDevicesmember shadows theMTConnect.Devicesnamespace inside the class scope, so the two pre-existing in-class references to that namespace are now fully qualified to keep their resolution unambiguous.tests/MTConnect.NET-HTTP-Tests/Clients/HttpClientDeviceModel.cs: new NUnit fixture pinning theDevicesaccessor (empty-before-probe, populated-after-probe, snapshot-not-live-view) and theDeviceReceivedevent (fires once per device per probe; the carriedIDevice's DataItems retain theirContainerandDeviceback-pointers — the wired-model invariant the issue calls out).The new tests are behavioural RED against the pre-fix source: the missing
Devicesproperty is a compile-error RED, the never-firedDeviceReceivedis a behavioural RED driven against the embeddedAgentRunnerHTTP server. Both go green with the fix in place; the existing fifteen HTTP client tests continue to pass.