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

feat: [UCR] Attribute protocol info in device #1287

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

dmocek
Copy link

@dmocek dmocek commented Nov 7, 2023

Attribute protocol information in the Device UCR.

@jpwhitemn
Copy link
Member

@dmocek - see my comment on PR #1288 as it is the answer to fixing the DCO and Semantic PR issue on this PR too.

docs_src/design/ucr/Protocol-Info-In-Device.md Outdated Show resolved Hide resolved
docs_src/design/ucr/Protocol-Info-In-Device.md Outdated Show resolved Hide resolved
Any users that create Device Profiles and Devices that have protocol-specific attribute values.

## Description
The Device Profile **describes** the device and its attributes, it's type. Different manufacturers can build the same type of device using different protocols and the same device using the same protocol but different configuration (e.g. different Modbus HoldingRegister's). For example, two different manufacturers may build an HVAC using ModBus, and another may build an HVAC using SNMP. In the case of two Modbus devices, there will need to be two different Device Profiles, even though the devices are the same and have the same attributes, because the protocol configurations (e.g. HoldingRegister) will conflict.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The Device Profile **describes** the device and its attributes, it's type. Different manufacturers can build the same type of device using different protocols and the same device using the same protocol but different configuration (e.g. different Modbus HoldingRegister's). For example, two different manufacturers may build an HVAC using ModBus, and another may build an HVAC using SNMP. In the case of two Modbus devices, there will need to be two different Device Profiles, even though the devices are the same and have the same attributes, because the protocol configurations (e.g. HoldingRegister) will conflict.
The Device Profile **describes** the device type and its attributes, it's type. Different manufacturers can build the same type of device using different protocols and the same device type using the same protocol but different configuration (e.g. different Modbus HoldingRegister's). For example, two different manufacturers may build an HVAC using ModBus, and another may build an HVAC using SNMP. In the case of two Modbus device types, there will need to be two different Device Profiles, even though the device types are the same and have the same attributes, because the protocol configurations (e.g. HoldingRegister) will conflict.

Copy link
Author

Choose a reason for hiding this comment

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

The first change (addition of 'type') was fixed. The last three are correct as-is.

  • '...and the same device using...' describes a manufacturer who built two of the same devices, but specified different configurations for the two.
  • The last two uses of device flagged also describe a manufacturer who build two of the same devices, but specified different configurations for the two.

docs_src/design/ucr/Protocol-Info-In-Device.md Outdated Show resolved Hide resolved
## Description
The Device Profile **describes** the device and its attributes, it's type. Different manufacturers can build the same type of device using different protocols and the same device using the same protocol but different configuration (e.g. different Modbus HoldingRegister's). For example, two different manufacturers may build an HVAC using ModBus, and another may build an HVAC using SNMP. In the case of two Modbus devices, there will need to be two different Device Profiles, even though the devices are the same and have the same attributes, because the protocol configurations (e.g. HoldingRegister) will conflict.

If the protocol information resides in the Device, which describes a specific (instance of a) device, only a single Device Profile is needed as all devices of the same type can use the same Device Profile. This becomes more important as you have more devices, potentially increasing the number and management of Device Profiles when a single one will do.
Copy link
Member

Choose a reason for hiding this comment

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

I am confused by this statement since this is how it is today (i.e. protocol in the device definition), but it doesn't support your need of a single Device Profile for all types of a Device.

Copy link
Author

Choose a reason for hiding this comment

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

I've clarified this.

-->

## Requirements
The requirement is to support the protocol-specific attribute values (e.g. HoldingRegister) in the Device as well as the Device Profile (for backward compatibility).
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The requirement is to support the protocol-specific attribute values (e.g. HoldingRegister) in the Device as well as the Device Profile (for backward compatibility).
The requirement is to support the protocol-specific attribute values (e.g. HoldingRegister) in the Device definition as well as the Device Profile (for backward compatibility).

Copy link
Member

Choose a reason for hiding this comment

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

Are you requesting that the Device definition's protocol be expanded for to support the more detail specifics (e.g. HoldingRegister) or the Device Profile be expanded with protocol specifics, which are currently in the Device definition?

Copy link
Author

Choose a reason for hiding this comment

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

The latter.

## Requirements
The requirement is to support the protocol-specific attribute values (e.g. HoldingRegister) in the Device as well as the Device Profile (for backward compatibility).

- If only the Device contains a protocol-specific attribute, it is used for that device.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- If only the Device contains a protocol-specific attribute, it is used for that device.
- If only the Device definition contains a protocol-specific attribute, it is used for that device.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed using suggestion.


- If only the Device contains a protocol-specific attribute, it is used for that device.
- If only the Device Profile contains a protocol-specific attribute, it is used for all Devices having that Device Profile.
- If both the Device Profile and the Device contain a protocol-specific attribute, the entry in the Device overrides the one in the Device Profile.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- If both the Device Profile and the Device contain a protocol-specific attribute, the entry in the Device overrides the one in the Device Profile.
- If both the Device Profile and the Device definition contain a protocol-specific attribute, the entry in the Device definition overrides the one in the Device Profile.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed using suggestion.

@lenny-goodell lenny-goodell changed the title feat: Attribute protocol info in device feat: [UCR] Attribute protocol info in device Nov 29, 2023
@lenny-goodell
Copy link
Member

@dmocek, you also need to add this UCR to the UCR TOC here: https://docs.edgexfoundry.org/3.1/design/TOC/
See this PR as example: #938

@dmocek
Copy link
Author

dmocek commented Dec 11, 2023

All issues have been addressed.

Comment on lines +30 to +35
The requirement is to support the protocol-specific attribute values (e.g. HOLDING_REGISTER's) in the Device definition as well as the Device Profile (for backward compatibility).

- If only the Device definition contains a protocol-specific attribute, it is used for that device.
- If only the Device Profile contains a protocol-specific attribute, it is used for all Devices having that Device Profile.
- If both the Device Profile and the Device definition contain a protocol-specific attribute, the entry in the Device definition overrides the one in the Device Profile.

Copy link
Member

Choose a reason for hiding this comment

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

The problem I see with this approach is the common protocol details in the device definition are repeated for every instance of a specific device type. Each time a device object is instantiated the common protocol details have to be known so they can be specified in the device definition. Currently the common protocol details are specified once in the Device Profile and the Define Definitions only contain the device instance specific protocol details

What if the Device Profile Resource attribute could be partition by protocol allowing the Device Services to pick the attributes base on the protocol name already specified in the Device definition.

Something like:

  name: "SwitchA"
  isHidden: true
  description: "On/Off , 0-OFF 1-ON"
  attributes:
    modbus-tcp:
      { primaryTable: "COILS", startingAddress: 0 }
    modbus-rtu:
      { primaryTable: "COILS", startingAddress: 1 }

Copy link
Member

Choose a reason for hiding this comment

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

Agree, the fomat of the profile resource attribute is free, so users can define multiple protocol information in the device resources. It's simpler than adding the protocol resource attribute to Device.

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.

4 participants