Wondering if we should keep a meta wrapper at this level which internally contains the unit definitions, instead of keeping units directly here, since units is just one aspect of the metadata.

Possibly. However, this can easily be added later when actually required.

Not quite sure about this case where we assume the value to be units, if we expect meta to be expanded beyond units in future. But, if we're pretty confident that unit is the most reasonable "default" meta for a measurement, then this is fine.

Yeah. Better to remove support for this shortcut. Otherwise we will have to support it sooner or later, even if not documented ;-)

Fixed: c7c208d

done 5b0032d

question: So, for a payload like {"unit: "u", "Temperature": {"unit": "°C"}, "Humidity": {"unit": "%RH"}} we only grab the "unit" and ignore the others?

I think we should only allow only one and print an error and don't add any units if both are present, ideally also adding this case to the tests. Or really, for any garbage data that can't be parsed published on these topics we should emit an ERROR event?

Also perhaps TRACE for when we actually set a new unit, but it's not as important.

There is a tension here between flexibility and adherence to thin-edge JSON conventions.

• We want the freedom to add not only units but also other info about measurements such as precision as with "temperature": { "unit": "°C", "precision": "±0.1°C" }
• We want to be able to use different units for "Temperature", "Climate": { "Temperature" } and "Engine": { "Temperature" }, e.g. with "Climate": { "Temperature": {"unit": "°C"} }

However, this makes things a bit complicated. As raised by @Bravo555, "Foo": {"unit: "u", "Temperature": {"unit": "°C"}} can then be understood as metadata assigning units to "Foo":20 values as well as "Foo" : { "Temperature": 20 } (the current code ignoring the latter, though).

An alternative is to flatten the structure of (measurement, info) pairs where group measurements are identified by a path.

 "Temperature": { "unit": "°C", "precision": "±0.1°C" },
 "Climate.Temperature": { "unit": "°C", "precision": "±0.1°C" },
 "Engine.Temperature": { "unit": "°F", "precision": "±1°F" },

=> I will give a try to this idea.

=> I will give a try to this idea.

7ddb8ed

question: If the messages have flat key-value structures, with no nesting, do we still need separate hashmaps for group units? Can't we put all the keys in a single hashmap?

Indeed, not required. This would simplify the mapping and its memory footprint. However this would also implies building paths (concatenating group and measurement names) each time a grouped measurement value is received.

@didier-wenzek Do you think this might introduce a flakey test if the meta topic is not processed before receiving the second temperature measurement?

If so, it might be worth doing two measurement assertions (one to check that the units are present, and the second to check that the units are not present)

@didier-wenzek Do you think this might introduce a flakey test if the meta topic is not processed before receiving the second temperature measurement?

Good point. The mapper sequentially processes all the measurement related messages. But the broker might change the message order as they are not published on the same topic.

Actually, not flaky.

$ invoke flake-finder --test-name "Measurement units can be cleared" --iterations 100 --outputdir output_ff --clean
------------------------------
Overall: PASSED
Results: 100 iterations, 100 passed, 0 failed
Elapsed time: 0:20:33.596564

I will remove that useless commit when re-basing 92c40f6

Yeah I was also surprised that it didn't seem to be flakey, though I pushed a slightly refactored test that also passed 50 flake finder runs (and the test removes the potential race condition)

commit: 40bea55