Platform Driver Development Framework (PDDF): Adding PDDF CLI utils #624
Conversation
|
Retest this please |
|
Retest this please |
2 similar comments
|
Retest this please |
|
Retest this please |
|
sonic-buildimage PR (sonic-net/sonic-buildimage#3387) has dependency on this PR. Please review this and provide comments so that the main PR can be approved and pushed. Thanks, |
pddf_fanutil/main.py
Outdated
| # ========================== Syslog wrappers ========================== | ||
|
|
||
|
|
||
| def log_info(msg, also_print_to_console=False): |
There was a problem hiding this comment.
We should avoid redundant code here, same log_* functions defined in several files, maybe you can consider to use a common log class: https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-daemon-base/sonic_daemon_base/daemon_base.py#L50
There was a problem hiding this comment.
Making changes as per your comments. The daemon_base.py class you mentioned can not be used in sonic-utilities as it is not part of HOST. So I added similar class
| @click.argument('index', type=click.STRING) | ||
| @click.argument('color', type=click.STRING) | ||
| @click.argument('color_state', type=click.STRING) | ||
| def setstatusled(device_name, index, color, color_state): |
There was a problem hiding this comment.
what's the motivation to expose a CLI to set status led? Shouldn't it be controlled by the system itself?
There was a problem hiding this comment.
Some are but some are not. Usually PSU1/PSU2/FAN are controlled by system itself, But LOC are controlled by user. CPLD spec has R/W permission on the control register. DIAG led too, is not controlled by system sometimes.
pddf_fanutil/main.py
Outdated
| # ==================== Methods for initialization ==================== | ||
|
|
||
| # Returns platform and HW SKU | ||
| def get_platform_and_hwsku(): |
There was a problem hiding this comment.
This function also defined in several files, would like to suggest to find a way to reduce redundant code. for the below load_platform_*util functions, although they are loading different device utilities, I believe can define one common function and load different device util according to input.
There was a problem hiding this comment.
Made changes as per your comments
|
@fk410167 |
|
restest this please |
|
retest default please |
|
retest this please |
1 similar comment
|
retest this please |
…624) This change is related to Platform Driver Development Framework (PDDF) which is being added to sonic-buildimage repo. More details can be found here, sonic-net/SONiC#406 PDDF supports its own CLI utilities, which use generic PDDF component plugins. I added these PDDF CLI utilities.
- What I did
This change is related to Platform Driver Development Framework (PDDF) which is being added to sonic-buildimage repo. More details can be found here,
sonic-net/SONiC#406
PDDF supports its own CLI utilities, which use generic PDDF component plugins. I added these PDDF CLI utilities.
- How I did it
Created pddf_psuutil, pddf_fanutil, pddf_thermalutil, pddf_ledutil.
- How to verify it
Under non-PDDF mode, these utilities won't run,
In PDDF mode, these utilities can be run with root privileges.
- Previous command output (if the output of a command-line utility has changed)
- New command output (if the output of a command-line utility has changed)
-->