Skip to content

fix(dc_measurements): constrain the Thermal Record's JSON schema - #351

Merged
Minipada merged 1 commit into
jazzyfrom
feature/300-thermal-measurement-emits-an-unvalidated
Aug 17, 2026
Merged

fix(dc_measurements): constrain the Thermal Record's JSON schema#351
Minipada merged 1 commit into
jazzyfrom
feature/300-thermal-measurement-emits-an-unvalidated

Conversation

@Minipada

Copy link
Copy Markdown
Owner

Closes #300

thermal.json declared a type: object with additionalProperties: {type: number} and
nothing else — {} validated, and so did a Record whose only "zone" was keyed "". Every
other Measurement's schema says something about what it emits; this one didn't.

The shape stays; only the schema changed

Zone type strings are the field names (x86_pkg_temp, cpu-thermal, …), platform-specific
and unknowable ahead of time, so there is no fixed properties/required list to write — which
is why the original schema punted. Draft-07 has the vocabulary for this case:

  • minProperties: 1 — at least one zone entry. For a map-shaped Record the entry is the
    field, so this is the "missing a required field" case. collect() already publishes nothing
    rather than a content-free {} when no zone can be read, so it can't fire on the plugin's own
    output: an empty Thermal Record reaching a Destination means something went wrong upstream.
  • propertyNames: {minLength: 1} — a zone with an empty type file can't become a nameless
    key (readZone() already rejects one).
  • additionalProperties: {type: number, minimum: -273.15} — degrees Celsius, above absolute
    zero, so a garbage temp read is caught rather than shipped.

base_path, zones, auto-discovery, the graceful-degradation contract and the emitted
{"<zone type>": <celsius>} map are untouched. With enable_validator off the schema is never
loaded (setValidationSchema() is already guarded on the flag), so behaviour there is unchanged.

Tests

3 → 8 in test_measurement_thermal.cpp. A ThermalSchema helper loads the installed
thermal.json via ament_index_cpp::get_package_share_directory() — the same file and the same
json_validator Measurement::validateJSON() uses, so the shipped artifact is what's exercised.

Test Covers
AcceptsARepresentativeRecord a two-zone Record validates
RejectsARecordWithNoZoneEntry {} fails
RejectsMalformedZoneEntries string temperature, sub-absolute-zero, empty zone key
PublishedRecordValidatesAgainstTheSchema end-to-end via MeasurementServer, validator on
PublishesTheSameRecordWithTheValidatorOff enable_validator: false, same Record

Worth knowing, since the first draft of the end-to-end test failed on it: enrichMsg() runs
validateJSON() first and only then adds name/nested/flattened/run_id/tags, so a
subscriber never sees the Record the validator saw. The test strips those framework keys — and
that ordering is why the schema can constrain values this tightly without breaking enrichment.

Verification

No colcon/ROS 2 in the dev sandbox, so this was built and tested for real in the cached
localhost/dc-workspace:latest Podman image with the worktree bind-mounted over
/root/ws/src/ros2_data_collection. colcon test --packages-select dc_measurements:
163 tests, 0 failures (8/8 thermal). The test log's INFO … schema: line confirms the new
schema is what the running plugin loads, and that Jazzy's nlohmann_json_schema_validator
implements propertyNames/minProperties — the one real unknown here. prek passes on all
changed files. doc/src/dc/measurements/thermal.md carries the new schema and a sentence on why
it's shaped this way rather than as a required list.

🤖 Generated with Claude Code

https://claude.ai/code/session_01BNt4fpZwLcgztWNjcLxAKf

thermal.json only said "object of numbers", so an empty Record, a nameless
zone key or a sub-absolute-zero temperature all passed validation. Zone type
strings are the field names, so there is no fixed `required` list to write;
draft-07's minProperties/propertyNames express the same invariants the plugin
already holds. Collection logic and the emitted Record shape are unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BNt4fpZwLcgztWNjcLxAKf
Signed-off-by: David Bensoussan <d.bensoussan@proton.me>
@codecov

codecov Bot commented Aug 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.54%. Comparing base (7062dfa) to head (a08311d).

Additional details and impacted files
@@           Coverage Diff           @@
##            jazzy     #351   +/-   ##
=======================================
  Coverage   67.54%   67.54%           
=======================================
  Files          95       95           
  Lines        5884     5884           
=======================================
  Hits         3974     3974           
  Misses       1910     1910           
Flag Coverage Δ
cpp-jazzy 67.54% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Minipada
Minipada merged commit 57fda1b into jazzy Aug 17, 2026
9 checks passed
@Minipada
Minipada deleted the feature/300-thermal-measurement-emits-an-unvalidated branch September 2, 2026 12:45
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.

1 participant