Skip to content

Conversation

ThatOneSix
Copy link

@ThatOneSix ThatOneSix commented Sep 3, 2025

Change Scope

  • This change adds a new component to the openconfig-platforms container for managing USB ports on devices. It is based on the logic used in openconfig-platform-transceiver.yang and augments /oc-platform:components/oc-platform:component. This is a continuation of PR #1339. It has been run through pyang lint successfully, with the following tree:
module: openconfig-platform-usb-port

  augment /oc-platform:components/oc-platform:component:
    +--rw usb-port
       +--rw config
       |  +--rw enabled?   boolean
       +--ro state
          +--ro enabled?   boolean

Also viewed as:

+--rw components
     +--rw component* [name]
         +--rw oc-usb-port:usb-port
               +--rw oc-usb-port:config
               |  +--rw oc-usb-port:enabled?   boolean
               +--ro oc-usb-port:state
                  +--ro oc-usb-port:enabled?   boolean
  • This change is backward compatible.

Platform Implementations

@dplore
Copy link
Member

dplore commented Sep 3, 2025

/gcbrun

@OpenConfigBot
Copy link

OpenConfigBot commented Sep 3, 2025

No major YANG version changes in commit b340803

@ThatOneSix
Copy link
Author

Resolved trailing whitespace issue that caused previous tests to fail.

Updated descriptions to be more general & reflect boolean statement rather than importing `power-admin-state`.
@dplore
Copy link
Member

dplore commented Sep 3, 2025

/gcbrun

@dplore dplore self-assigned this Sep 4, 2025
@dplore
Copy link
Member

dplore commented Sep 4, 2025

/gcbrun

@dplore dplore moved this to last-call in OC Operator Review Sep 4, 2025
@dplore dplore moved this from last-call to Waiting for author in OC Operator Review Sep 4, 2025
@dplore
Copy link
Member

dplore commented Sep 4, 2025

Reviewed in September 4, 2025 OC Community meeting without objections. I'll wait to move this to last call until @ThatOneSix has reviewed this with more implementors. @ThatOneSix let me know when you have internal sign off on this approach and we can move it to a last call status and then merge.

@LimeHat
Copy link

LimeHat commented Sep 8, 2025

The tree in the PR description implies that each component may have a usb-port container attached to it which controls an individual port (or a set of ports) associated with the component.

At the same time, the PR content shows that you introduce a new component type identity, which implies that a port is modelled as a separate entity.

Could you please clarify this? Is your intention to support both models (usb ports as separate components and usb ports as a property of other components)?

@dplore dplore closed this Sep 8, 2025
@dplore dplore reopened this Sep 8, 2025
@dplore
Copy link
Member

dplore commented Sep 8, 2025

/gcbrun

@ThatOneSix
Copy link
Author

ThatOneSix commented Sep 8, 2025

The tree in the PR description implies that each component may have a usb-port container attached to it which controls an individual port (or a set of ports) associated with the component.

At the same time, the PR content shows that you introduce a new component type identity, which implies that a port is modelled as a separate entity.

Could you please clarify this? Is your intention to support both models (usb ports as separate components and usb ports as a property of other components)?

My intent is to model USB ports as their own components.

To view the tree another way:

+--rw components
     +--rw component* [name]
         +--rw oc-usb-port:usb-port
               +--rw oc-usb-port:config
               |  +--rw oc-usb-port:enabled?   boolean
               +--ro oc-usb-port:state
                  +--ro oc-usb-port:enabled?   boolean

So it is on the same level as something like fan or cpu.

@nkitchen-arista
Copy link
Contributor

Could you please clarify this? Is your intention to support both models (usb ports as separate components and usb ports as a property of other components)?

In the component model, it's typical for a component of type XYZ to have its type-specific data in a subcontainer with a corresponding name xyz. Usually components of other types won't populate the container. For example, we would only expect components of type TRANSCEIVER to have /components/component/transceiver.

The YANG modules don't contain constraints to enforce this, but it doesn't mean that we expect to have containers named usb-port as children of arbitrary components.

@LimeHat
Copy link

LimeHat commented Sep 9, 2025

My intent is to model USB ports as their own components.

Is there a specific reason behind this choice?
I would argue that inlining usb ports to the parent components is an easier approach to work with, considering that they have only one property.

but it doesn't mean that we expect to have containers named usb-port as children of arbitrary components.

Only the ones that have such ports :-). There's nothing in OC community that says this is invalid, it is a choice vendors can make.

@dplore
Copy link
Member

dplore commented Sep 9, 2025

My intent is to model USB ports as their own components.

Is there a specific reason behind this choice? I would argue that inlining usb ports to the parent components is an easier approach to work with, considering that they have only one property.

Can you give an example of what you are suggesting regarding "inlining usb ports to the parent components"?

but it doesn't mean that we expect to have containers named usb-port as children of arbitrary components.

Only the ones that have such ports :-). There's nothing in OC community that says this is invalid, it is a choice vendors can make.

Yes, this flexibility is intentional and up to implementors to decide what relationship is appropriate for their hardware. There are some limited expectations encoded into featureprofiles tests. (one example)

@LimeHat
Copy link

LimeHat commented Sep 9, 2025

Can you give an example of what you are suggesting regarding "inlining usb ports to the parent components"?

Just placing usb-port container directly under the parent component, e.g.
/components/component[name=Supervisor1]/usb-port/{config,state}/enabled

instead of creating a new component with a new identity, such as /components/component[name=USBPort-Supervisor1]/usb-port/{config,state}/enabled.

I don't think either approach is wrong, but the latter seems slightly excessive. But if the community prefers to have an identity as well, that's fine.

@earies
Copy link
Contributor

earies commented Sep 10, 2025

Can you give an example of what you are suggesting regarding "inlining usb ports to the parent components"?

Just placing usb-port container directly under the parent component, e.g. /components/component[name=Supervisor1]/usb-port/{config,state}/enabled

instead of creating a new component with a new identity, such as /components/component[name=USBPort-Supervisor1]/usb-port/{config,state}/enabled.

I don't think either approach is wrong, but the latter seems slightly excessive. But if the community prefers to have an identity as well, that's fine.

Didn't realize another PR was opened for this - same comment on prior PR #1339

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Waiting for author
Development

Successfully merging this pull request may close these issues.

6 participants