Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updating top-level class relationships for parity #47

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

rszcodronski
Copy link
Collaborator

I have updated the top-level classes with relationships to provide parity across all top-level classes such as:

  • hasDocument
  • includedIn
  • locatedIn (BuildingComponent)

Copy link
Contributor

@alinamstanciu alinamstanciu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@hammar hammar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a bunch of individual comments to the respective changes.

Overall: these are interesting and useful developments, but perhaps worth taking an iteration or two more on getting right, and ensuring we don't reinvent the wheel compared w/ the upstream models.

I'm going on vacation soon (in 55 minutes to be precise) but will keep an eye on the email while I'm gone. Back in mid-August. If this is urgent to resolve before then, I'm sure I can find time for a conference call.

},
{
"@type": "Relationship",
"name": "hasDocument",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the semantics of an agent having a document? Does that imply that the agent created the document, manages the document, owns the document, utilizes the document...?

},
{
"@type": "Relationship",
"name": "isFedBy",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the semantics of a building component (which is supposed to a piece of the buildings structural makeup) being fed some substance? How does a slab of concrete, or a wall, get fed substances?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The isFedBy relationship is more for classes in other disciplines which aren't yet defined in REC such as HVAC Ducts, Electrical Circuits, and Plumbing piping but have been defined in Willow here.

@@ -42,6 +42,20 @@
},
"name": "isCapabilityOf"
},
{
"@type": "Relationship",
"name": "hostedBy",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I disagree with this. The hostedBy property is used to indicate a logical component (e.g., a piece of software) executing on an asset (a piece of hardware). For capabilities, what is the difference between "hostedBy" and "isCapabilityOf"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question - the difference is meant to define a common scenario where a Capability is hosted by a Controller/Gateway in terms of where you actually retrieve its data but it is actually a capability of another asset/space. We have a metering sample here and an HVAC sample here which depict this scenario.

},
{
"@type": "Relationship",
"name": "isControlledBy",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are all capabilities controllable? Or only a subset?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we think of Capabilities as either Inputs (Sensor, Setpoint, State), Variables (Parameters), or Outputs (Actuators) to a controller, then you could say that only the Variables and Outputs can be "controlled" by another Capability.

@@ -2,6 +2,21 @@
"@id": "dtmi:digitaltwins:rec_3_3:core:Document;1",
"@type": "Interface",
"contents": [
{
"@type": "Relationship",
"name": "isDocumentOf",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where I'd prefer to use the notion of "topic" or somesuch, if that is indeed what it is supposed to cover; "isDocumentOf" is rather vague.?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the upstream OWL models I'm calling this "documentTopic", to indicate that the relationship doesn't imply ownership but rather a subject/topic relationship. I hope that is OK.

"displayName": {
"en": "Produced By"
},
"name": "producedBy"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In REC-OWL we actually have "Observation" as subclass of Event, which in turn has a property "observationGeneratedBy" pointing at a Sensor. This was filtered out from DTDL REC because it seemed to be out-of-scope to the ADT/DTDL/Willow use cases. If such a linkage is now of interest, I'd propose we remove that filter and include the upstream properties instead. OK?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this has a little different intended use. "Produced By" is meant to be used by events which I wouldn't call "Observations" since they are being produced by an active system/person such as a "Card Swipe Event" or "Elevator Trip Event". The use of Event enables scenarios where a system is producing telemetry that is a little more complex than Capability which has a simple value (boolean or number). I decided on the term "produce" because this is what the Event-driven architectures use such as Kafka and Event Hub.

@hammar
Copy link
Member

hammar commented Nov 30, 2021

I updated this PR from master. Since we need to ship REC-OWL 3.3, and I would like the OWL and DTDL models to be consistent, I'm going to try and update this PR as best I can in accordance with the commentary above, and hopefully I don't be stepping too much on anyone's toes.

@hammar
Copy link
Member

hammar commented Nov 30, 2021

See updates above. I'm OK with merging, or with continuing the discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants