Refactor Pcied and add unittest#189
Conversation
|
This pull request introduces 2 alerts when merging 3b20d27 into 9297a29 - view on LGTM.com new alerts:
|
sonic-pcied/scripts/pcied
Outdated
| def _wrapper_get_pcie_check(): | ||
| return platform_pcieutil.get_pcie_check() | ||
|
|
||
| def _wrapper_get_pcie_aer_stats(bus, dev, func): | ||
| return platform_pcieutil.get_pcie_aer_stats(bus=bus, device=dev, func=func) |
There was a problem hiding this comment.
It seems like these wrappers are unnecessary because you aren't checking for old or new API. The same function is being called for both, so these seem superfluous.
|
This pull request introduces 1 alert when merging 8a47121 into bf60a27 - view on LGTM.com new alerts:
|
|
This pull request introduces 1 alert when merging 61f07ed into bf60a27 - view on LGTM.com new alerts:
|
| self.aer_stats = {} | ||
| if Id is not None: | ||
| self.device_table.set(self.device_name, [('id', Id)]) | ||
| self.aer_stats = platform_pcieutil.get_pcie_aer_stats(bus=Bus, dev=Dev, func=Fn) |
There was a problem hiding this comment.
Please update the argument names as specified in https://github.com/Azure/sonic-platform-common/blob/master/sonic_platform_base/sonic_pcie/pcie_common.py#L102
| self.aer_stats = platform_pcieutil.get_pcie_aer_stats(bus=Bus, dev=Dev, func=Fn) | |
| self.aer_stats = platform_pcieutil.get_pcie_aer_stats(bus=Bus, device=Dev, func=Fn) |
sonic-pcied/scripts/pcied
Outdated
| if Id is not None: | ||
| self.device_table.set(self.device_name, [('id', Id)]) | ||
| self.aer_stats = _wrapper_get_pcie_aer_stats(bus=Bus, dev=Dev, func=Fn) | ||
| self.aer_stats = platform_pcieutil.get_pcie_aer_stats(bus=Bus, device=Dev, func=Fn) |
There was a problem hiding this comment.
Why change from dev to device? It looks like the parameter is called dev: https://github.com/Azure/sonic-platform-common/blob/master/sonic_platform_base/sonic_pcie/pcie_base.py#L44
There was a problem hiding this comment.
It is observed that the parameter name is dev in the base class method defined in pcie_base.py, while it is overridden as device by the derived class method in pcie_common.py used by pcied (https://github.com/Azure/sonic-platform-common/blob/master/sonic_platform_base/sonic_pcie/pcie_common.py#L104) unifying them both as dev would resolve the issue
There was a problem hiding this comment.
Updated the pcie_common.py with this PR : sonic-net/sonic-platform-common#197
And revert the change in pcied.
Unifying the platform api instead
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
…able name to match with pcied changes (#7886) Why I did it Support multiple pcie configuration file and change the pcie status table name This is to match with below two PRs. sonic-net/sonic-platform-common#195 sonic-net/sonic-platform-daemons#189 How I did it Check pcie configuration file with wild card and change the device status table name How to verify it Restart with changes and see if the pcie check works as expected.
…able name to match with pcied changes (#7886) Why I did it Support multiple pcie configuration file and change the pcie status table name This is to match with below two PRs. sonic-net/sonic-platform-common#195 sonic-net/sonic-platform-daemons#189 How I did it Check pcie configuration file with wild card and change the device status table name How to verify it Restart with changes and see if the pcie check works as expected.
|
@jleveque Can you please review and approve this PR? PR build passed now. Thanks! |
|
This commit could not be cleanly cherry-pick to 202012. Please submit another PR. |
Description Refactor the pcied and add the unit test Motivation and Context Added unit test to increase the pmon unit test coverage. How Has This Been Tested? Build with unit test enabled and run manually on a dut to verify the pcied.
|
This PR could not be cleanly cherry-picked to 202012. Please submit another PR. |
…able name to match with pcied changes (sonic-net#7886) Why I did it Support multiple pcie configuration file and change the pcie status table name This is to match with below two PRs. sonic-net/sonic-platform-common#195 sonic-net/sonic-platform-daemons#189 How I did it Check pcie configuration file with wild card and change the device status table name How to verify it Restart with changes and see if the pcie check works as expected.
…t#189) Description Added constants to component_base for each potential return code of auto_firmware_update() so that vendor specific component implementations may reference these to provide the correct return codes to the firmware auto update utility. Motivation and Context This provides a central location from which standardized return codes may be imported, preventing the need for potential inconsistency in vendor implementations or the use of magic numbers in vendor implementations for return code. How Has This Been Tested? Non-functional change, builds successfully and can be successfully imported from at runtime.
4 participants
Description
Refactor the pcied and add the unit test
Motivation and Context
Added unit test to increase the pmon unit test coverage.
How Has This Been Tested?
Build with unit test enabled and run manually on a dut to verify the pcied.
Additional Information (Optional)