-
Notifications
You must be signed in to change notification settings - Fork 672
Adding platform model for USB port #1339
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
base: master
Are you sure you want to change the base?
Conversation
Model to support enable/disable of USB ports on hardware such as Access Points.
/gcbrun |
No major YANG version changes in commit 099e9d2 |
|
||
leaf enabled { | ||
type boolean; | ||
default "false"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OC prefers to not define a default value, unless there is some industry standard we can refer to that requires this and is widely implemented. This means any configuration needs to specify what the desire is and that there is no openconfig specification of what a 'factory default' setting must be (which is often problematic).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack. I will remove the default value.
identity USB_PORT { | ||
base OPENCONFIG_HARDWARE_COMPONENT; | ||
description | ||
"USB port, typically USB-A."; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should make this more strict. Is this intended to be used for any and all USB port types? Or if we're expecting only USB-A, then we'll need something more explicit to identify the types you want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would we need to mention connector types at all here? This seems irrelevant and could come in many flavors - in fact we already have many connector types across network element platforms when it comes to USB for console, storage devices, etc..
@@ -0,0 +1,86 @@ | |||
module openconfig-platform-usb-port { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few general comments:
- Can you articulate what your specific intentions are with respect to conveying a port-type such as USB? Will we start to define specific types for everything else? Is this predominantly for serial (console, aux) communications, block storage devices, etc..? How about non-USB serial, etc..?
- Why would this need to be a self-contained model vs. expansion of existing?
- If for serial purposes, then what about all other relevant settings?
- If for storage devices, then how might this component relate to a storage component?
identity USB_PORT { | ||
base OPENCONFIG_HARDWARE_COMPONENT; | ||
description | ||
"USB port, typically USB-A."; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would we need to mention connector types at all here? This seems irrelevant and could come in many flavors - in fact we already have many connector types across network element platforms when it comes to USB for console, storage devices, etc..
Change Scope
Adding new platform model for USB ports, enable/disable functionality.
This change is backwards compatible
Platform Implementations
Mobility Controller-based Aruba Access Points: https://arubanetworking.hpe.com/techdocs/CLI-Bank/Content/aos8/ap-system-pro.htm (
ap-usb-power-mode [enable|disable]
)Cisco Catalyst switches: https://www.cisco.com/c/en/us/td/docs/switches/lan/catalyst9400/software/release/17-17/command_reference/b_1717_9400_cr/interface_and_hardware_commands.html?dtid=osscdc000283&linkclickid=srch#wp3775305635 (
[no] platform usb disable
)