Azure / opendigitaltwins-building

Open Digital Twins Definition Language (DTDL) RealEstateCore Ontology
MIT License
157 stars 44 forks source link

Defining property lastValue higher in the hierarchy? #68

Open mwdhont opened 2 years ago

mwdhont commented 2 years ago

Hi all,

I believe it would make sense to define the property "lastValue" higher in the inheritance hierarchy. More specifically, is there a good reason to not define "lastValue" on the "Capability"-model? In the end, what is the purpose of having a property "lastValueTime" without having the option to store/reference that lastValue?

I believe this approach would make more sense than the current 127 references where "lastValue" is currently defined: https://github.com/Azure/opendigitaltwins-building/search?q=lastValue&type=code

A related question on this topic was discussed before in https://github.com/Azure/opendigitaltwins-building/issues/42.

Best, Michiel

hammar commented 2 years ago

The reason we're defining this on a lower level is that it allows different schemas (and potentially different units) for different sensors. If we defined it in a single place (on Capability) we'd have to fall back to a shared schema representation at that definition point, i.e., a string representation for all kinds of sensor values, which is probably a bit too vague. The lastValueTime does not have that same problem: it'll always be a timestamp.

Admittedly this does make for a somewhat confusing model on the Capability level. Let's keep this issue (and discussion) alive and see if we can get more input on it, perhaps worth taking another go on it at some point.

Thanks for bringing this up!

Karl

mwdhont commented 2 years ago

Hi Karl,

Thanks for your reply and clarification.

However, I believe there are still some gaps to be filled. To give one example: PowerSensor inherits from QuantitySensor which inherits from Sensor and from their up to Capability. In this whole inheritance chain, no ‘lastValue’ is defined.

An ideal scenario would be to be able to define ‘lastValue’ on the highest level and to specify the unit on the lower levels, if you need to define this unit.

Regards, Michiel

hammar commented 2 years ago

Hi,

PowerSensor has no lastValue because its children have diverging lastValue schemas.

Unfortunately the design you propose, while it makes logical sense to me, does not work with how DTDL handles subclassing: fields on parent classes cannot be overloaded/hidden by fields on children. The DTDL spec document does not cover this case explicitly, but the DTDL validator fails in such cases.

Karl

mwdhont commented 2 years ago

Hi Karl,

Thanks for clarifying. My bad, if you go more specific you are indeed able to define a lastValue + unit. I guess by going specific enough, the user will always be able to define a lastValue?

Michiel

hammar commented 2 years ago

Yes, that's the idea. If we've missed any such cases, then that would be a fault.

Best,

Karl