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

dc_measurements/plugins/measurements/json/thermal.json

@@ -1,10 +1,16 @@
 {
   "$schema": "http://json-schema.org/draft-07/schema#",
   "title": "Thermal",
-  "description": "Temperature readings from /sys/class/thermal, keyed by each zone's type string",
+  "description": "One entry per thermal zone read this cycle, keyed by the zone's type string",
+  "propertyNames": {
+    "description": "The zone's type string, e.g. x86_pkg_temp or cpu-thermal",
+    "minLength": 1
+  },
   "additionalProperties": {
     "description": "Temperature of the zone in degrees Celsius",
-    "type": "number"
+    "type": "number",
+    "minimum": -273.15
   },
+  "minProperties": 1,
   "type": "object"
 }

dc_measurements/test/test_measurement_thermal.cpp

@@ -4,13 +4,44 @@
 #include <chrono>
 #include <filesystem>
 #include <fstream>
+#include <nlohmann/json-schema.hpp>
 #include <string>
 #include <thread>
 
+#include "ament_index_cpp/get_package_share_directory.hpp"
 #include "dc_interfaces/msg/string_stamped.hpp"
 #include "dc_measurements/measurement_server.hpp"
 #include "dc_util/json_utils.hpp"
 
+// Runs a Record through the Measurement's own installed schema, the same file and validator
+// `Measurement::validateJSON()` loads when `enable_validator` is on.
+class ThermalSchema
+{
+public:
+  ThermalSchema()
+  {
+    std::ifstream schema_file(ament_index_cpp::get_package_share_directory("dc_measurements") +
+                              "/plugins/measurements/json/thermal.json");
+    validator_.set_root_schema(nlohmann::json::parse(schema_file));
+  }
+
+  bool accepts(const nlohmann::json& record)
+  {
+    try
+    {
+      validator_.validate(record);
+      return true;
+    }
+    catch (const std::exception&)
+    {
+      return false;
+    }
+  }
+
+private:
+  nlohmann::json_schema::json_validator validator_;
+};
+
 // Builds a fake /sys/class/thermal-shaped directory tree under /tmp so the plugin can be
 // exercised for real (auto-discovery, zone `type` as Record key, ARM-style non-numeric-suffix
 // naming) without depending on whatever thermal zones (if any) the test host/container exposes.
@@ -172,6 +203,66 @@ TEST_F(MeasurementThermalTest, ActivatesSuccessfullyWithMissingBasePath)
   SUCCEED();
 }
 
+TEST(ThermalSchemaTest, AcceptsARepresentativeRecord)
+{
+  ThermalSchema schema;
+  EXPECT_TRUE(schema.accepts(nlohmann::json{ { "x86_pkg_temp", 52.0 }, { "gpu-thermal", 61.5 } }));
+}
+
+TEST(ThermalSchemaTest, RejectsARecordWithNoZoneEntry)
+{
+  // Zone type strings *are* the field names, so the entry itself is what's required: a Record
+  // with none carries no reading at all. The Measurement never emits one (it publishes nothing
+  // that cycle instead), so an empty Record reaching a Destination means something went wrong.
+  ThermalSchema schema;
+  EXPECT_FALSE(schema.accepts(nlohmann::json::object()));
+}
+
+TEST(ThermalSchemaTest, RejectsMalformedZoneEntries)
+{
+  ThermalSchema schema;
+  EXPECT_FALSE(schema.accepts(nlohmann::json{ { "cpu-thermal", "45.1" } }));
+  EXPECT_FALSE(schema.accepts(nlohmann::json{ { "cpu-thermal", -400.0 } }));
+  EXPECT_FALSE(schema.accepts(nlohmann::json{ { "", 45.1 } }));
+}
+
+TEST_F(MeasurementThermalTest, PublishedRecordValidatesAgainstTheSchema)
+{
+  FakeThermalTree tree;
+  tree.addZone("thermal_zone0", "x86_pkg_temp", 52000);
+
+  declareCommonParameters();
+  ms_node_->declare_parameter("thermal.base_path", tree.path());
+
+  startLifecycleNode();
+  spinUntilCallback();
+
+  // publish() enriches the Record *after* validateJSON() has run, so strip what the framework
+  // added to get back the Record the validator actually saw.
+  nlohmann::json record = data_json_;
+  for (const char* enrichment_key : { "name", "plugin", "nested", "flattened", "run_id", "tags" })
+  {
+    record.erase(enrichment_key);
+  }
+
+  EXPECT_TRUE(ThermalSchema().accepts(record));
+}
+
+TEST_F(MeasurementThermalTest, PublishesTheSameRecordWithTheValidatorOff)
+{
+  FakeThermalTree tree;
+  tree.addZone("thermal_zone0", "cpu-thermal", 45123);
+
+  declareCommonParameters();
+  ms_node_->declare_parameter("thermal.base_path", tree.path());
+  ms_node_->declare_parameter("thermal.enable_validator", false);
+
+  startLifecycleNode();
+  spinUntilCallback();
+
+  EXPECT_DOUBLE_EQ(data_json_["cpu-thermal"].get<double>(), 45.123);
+}
+
 int main(int argc, char** argv)
 {
   ::testing::InitGoogleTest(&argc, argv);

doc/src/dc/measurements/thermal.md

@@ -21,15 +21,26 @@ empty Record.
 
 ## Schema
 
+Zone type strings are the field names, so they can't be listed ahead of time — the schema
+constrains their shape instead: at least one entry (the Measurement publishes nothing rather
+than an empty Record), a non-empty type string as key, and a temperature in degrees Celsius
+above absolute zero as value.
+
 ```json
 {
     "$schema": "http://json-schema.org/draft-07/schema#",
     "title": "Thermal",
-    "description": "Temperature readings from /sys/class/thermal, keyed by each zone's type string",
+    "description": "One entry per thermal zone read this cycle, keyed by the zone's type string",
+    "propertyNames": {
+        "description": "The zone's type string, e.g. x86_pkg_temp or cpu-thermal",
+        "minLength": 1
+    },
     "additionalProperties": {
         "description": "Temperature of the zone in degrees Celsius",
-        "type": "number"
+        "type": "number",
+        "minimum": -273.15
     },
+    "minProperties": 1,
     "type": "object"
 }
 ```

progress.txt

@@ -6634,3 +6634,66 @@ package carrying all three placeholder fields: each is reported, and both checks
 the workspace agrees (single-quoted `version='…'` in setup.py included, which upstream's
 `grep` did not accept). Not run locally: the `build-doc` hook, unchanged by this work and
 covered by `doc.yaml` in CI.
+
+## #300 - Give the Thermal Measurement a schema that actually constrains its Record
+
+`thermal.json` declared a `type: object` with an `additionalProperties: {type: number}` and
+nothing else — no floor on how few entries a Record may carry, no constraint on the keys, and
+no bound on the values. `{}` validated, and so did a Record whose only "zone" was named `""`.
+Every other Measurement's schema says something about what it emits; this one didn't.
+
+**The shape stays as it is; 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 exactly this case, and the new schema uses it:
+
+- `minProperties: 1` — a Record must carry at least one zone entry. This is the "missing a
+  required field" case for a map-shaped Record: the entry *is* the field. `collect()` already
+  returns an empty `StringStamped` (publishing nothing that cycle) rather than a content-free
+  `{}` when no zone can be read, so this 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 whose `type` file is empty can't become a nameless
+  key. `readZone()` already rejects an empty `type`, so again the schema documents and enforces
+  an invariant the plugin holds rather than inventing a new one.
+- `additionalProperties: {type: number, minimum: -273.15}` — degrees Celsius, above absolute
+  zero. A garbage `temp` read (or a units mix-up putting millidegrees through unconverted in
+  the negative direction) is caught instead of shipped.
+
+Nothing about collection changed: `base_path`, `zones`, auto-discovery, the graceful-degradation
+contract and the emitted `{"<zone type>": <celsius>}` map are all untouched, and with
+`enable_validator` off the plugin never loads the schema at all (`setValidationSchema()` is
+already guarded on the flag) so its behaviour is unchanged either way.
+
+**Tests** (`test_measurement_thermal.cpp`, 3 existing → 8). A `ThermalSchema` helper loads the
+*installed* `thermal.json` through `ament_index_cpp::get_package_share_directory()` — the same
+file and the same `nlohmann::json_schema::json_validator` `Measurement::validateJSON()` uses, so
+the test exercises the shipped artifact rather than a copy of it:
+
+- `AcceptsARepresentativeRecord` — a two-zone Record validates.
+- `RejectsARecordWithNoZoneEntry` — `{}` fails (`minProperties`).
+- `RejectsMalformedZoneEntries` — a string temperature, a sub-absolute-zero temperature, and an
+  empty zone-type key each fail.
+- `PublishedRecordValidatesAgainstTheSchema` — end-to-end through the `MeasurementServer`
+  harness with the validator on (its default), against a `FakeThermalTree`.
+- `PublishesTheSameRecordWithTheValidatorOff` — `enable_validator: false`, same Record.
+
+Worth recording, because the first draft of the end-to-end test failed on it: `publish()` calls
+`enrichMsg()`, which runs `validateJSON()` **first** and only then adds `name`, `nested`,
+`flattened`, `run_id` and `tags`. So the Record a subscriber sees is not the one the validator
+saw, and a schema with a closed `additionalProperties` would reject it — the test strips those
+framework keys before validating. This is why the schema can constrain values as tightly as it
+does without breaking the enrichment path.
+
+**Verified for real**, not just linted: no colcon/ROS 2 in this sandbox (same recurring
+constraint as the rest of this log), so the cached `localhost/dc-workspace:latest` Podman image
+was reused with this worktree bind-mounted over `/root/ws/src/ros2_data_collection` — the image
+was five days stale and missing `dc_common/file_scratch_ring.hpp`, so the whole tree had to be
+mounted and `colcon build --packages-up-to dc_measurements` re-run, not just `dc_measurements`.
+`colcon test --packages-select dc_measurements`: 8/8 thermal tests pass, and the whole package
+suite is green at 163 tests / 0 failures. The
+`INFO … schema:` line in the test log confirms the new schema is what the running plugin loads,
+and that `nlohmann_json_schema_validator` in Jazzy implements `propertyNames`/`minProperties`
+(it does — the three negative cases all fail as intended, which was the one real unknown here).
+`prek run --files …` passes on the three changed files. `doc/src/dc/measurements/thermal.md`'s
+Schema section carries the new schema plus a sentence on why the constraints are shaped this
+way instead of a `required` list.