Discussion / suggestion

[joakim-hove]

I feel there is "something rotten" in the datamodel with (at least) the concepts of Device, Sensor, MeasurementType and SensorType. In particular the pattern that sensors have id as:

sensor_id = "%s_%s" % (device_id , measurement_type)

has become a hard-wired assumption which is not reflected anywhere in the datamodel - bad bad. It is not clear to me what is the Right™ way to improve on this, but as I see it the fundamental entities are:

Devices: These have a physical location, an owner and so on.

MeasurementTypes: I.e. are we focusing on PM10 or PM25 - or temperature for that matter (maybe SensorType actually is a better choice?)

Based on that assumption I have changed the post code to accept id = (device_id, mtype) instead of sensor_id. Observe that this is very much just a first step:

1. The code still accepts sensor_id in the POST payload, i..e there is no need to immediately update the client.

2. The (dev_id, mtype) pair is immediately converted to a sensor_id - if we decide to go along this way we should of course let the change permeate deeper in the code.

I guess the current datamodel is the fully flexible solution, but currently we do not use that flexibility, and instead there are other parts of the code which actually assume conventions like the mentioned structure of the sensor_id - iff we do not foresee that we will ever use the full flexibility it might be wise limit the flexibility somewhat and embed those constraints explicitly in the datamodel?

[pgdr]

I completely agree with most of that, and I fully support the move to posting (device, mtype) instead of sensor_id.

Actually, I agree with everything, but I would like to comment: A measurement type is specific thing we measure; it has a unit, and a description of what's measured. In the case of PM10, the unit is \mu g/m^3 and the thing measured is particulate matter of size 10µm, whereas in TEMP, it is temperature that is being measured and the unit is K or degrees C. I think we agree here.

However, in the future, we might add a sensor ID. We currently use the type SDS011 to measure PM10, but NILU have other sensors. We don't care about that now, but maybe in the future we will. We'll deal with that later.

I suggest we delete the sensor table and keep the backwards compatible API until all our devices are running the new pip-based python-friskby.

[joakim-hove]

A measurement type is specific thing we measure; it has a unit, and a description of what's measured. In the case of PM10, the unit is \mu g/m^3 and the thing measured is particulate matter of size 10µm, whereas in TEMP, it is temperature that is being measured and the unit is K or degrees C. I think we agree here

We absolutely agree - the things you mention are in the SensorType model. The SensorTypeand MeasurementType are split so that you can say: "Give me all measurments of temperature - irrespective of the characteristics of the sensor used`.

The data model was "designed" a bit in the blind, with an ambition to cover all eventualities, and with a fear that it would be difficult to improve/change later. Experience has shown that Django Migration Work™ - and that changing the data model is quite simple.

So - maybe the MeasurementType and SensorType models should be united?

I suggest we delete the sensor table and keep the backwards compatible API until all our devices are running the new pip-based python-friskby.

I agree

[pgdr]

I will take the client side; do you deal with server side?

[joakim-hove]

OK - I have addressed the stylistic comments. Merging when Travis is green

[joakim-hove]

OK - I will continue the work towards getting rid of the Sensor model; but I still consider this:

So - maybe the MeasurementType and SensorType models should be united?

a relevant question.

[pgdr]

maybe the MeasurementType and SensorType models should be united?

Yes; if we can't simply delete SensorType.