search

Azure / opendigitaltwins-building

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

Updated capabilities from Willow #39

Closed hammar closed 3 years ago

hammar commented 3 years ago

Note that the display names are automatically generated from the local names using a regex and some string processing McGyvering; so in some cases they may not be 100 %. I'd be happy to evolve the upstream models on this as needed, but given time pressure, think it's most important to get this out; we can iterate on the display names later.

hammar commented 3 years ago

Looking great! Here are the items I found in review:

All Capabilty Subclasses

  • TimeSpan - add to the list of Quantities. This differs from Time in that Time is 1:00PM (what a clock shows) whereas TimeSpan is 1 Hour
  • StartLevel - remove FanRunLevel, RunLevel, VFRunLevel to keep consistent with not defining specific components and aligning with just using the term Start and Run as a sameAs.
  • OnLevel - should we remove CoolingLevel and HeatingLevel since these are specific HVAC Modes which I've punted on for now or just leave for now? There are many more HVAC mode concepts such as Economizing, Standby, Leadlag but other types of equipment/systems may have many more
  • Present/Absent vs Presence/Absence - we (me) had interchanged these across Sensor/Actuator/Setpoint vs State. Should we align across all Capability sub-classes? I have a slight preference for Presence/Absence which I think uses similar grammar to other BinaryState classes.

State

  • LeakState - remove this from the State class as it only applies as LeakSensor within the Sensor\State hieararchy like you have done with FireSensor, SmokeSensor, etc.

Thanks, good comments, will implement and commit.

hammar commented 3 years ago

@rszcodronski How about those binaries under Setpoint, e.g., "LeakSetpoint", "FireSetpoint", etc. -- should these also be removed? My intuition is yes, but I'm a little out of my depth.

rszcodronski commented 3 years ago

@hammar Yes, I agree that the binaries under Setpoint such as LeakSetpoint and FireSetpoint should also be removed as these really only make sense under Sensor.