-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[device]: Fix a bug that psuutil cannot access gpio sysfs to get PSU status #1789
Conversation
try: | ||
with open(export_file, 'w') as fd: | ||
fd.write(str(gpio_base+pinnum)) | ||
except Exception as error: | ||
logging.error("Unable to export gpio ", pinnum) | ||
logging.error("Unable to export gpio ", str(gpio_base+pinnum)) | ||
sys.exit(0) |
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 exit 0 here? the plugin should not do exit
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.
@lguohan OK I'll remove it
@@ -54,6 +67,7 @@ def read_psu_statuses(self, pinnum): | |||
retval = fd.read() | |||
except Exception as error: | |||
logging.error("Unable to open ", gpio_file, "file !") | |||
sys.exit(0) |
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.
the plugin should not do exit.
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.
@lguohan OK I'll remove it
try: | ||
with open(export_file, 'w') as fd: | ||
fd.write(str(gpio_base+pinnum)) | ||
except Exception as error: | ||
logging.error("Unable to export gpio ", pinnum) | ||
logging.error("Unable to export gpio ", str(gpio_base+pinnum)) |
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.
This is invalid syntax. Please change to
logging.error("Unable to export gpio " + str(gpio_base+pinnum))
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.
OK, I will change it.
except Exception as error: | ||
logging.error("Unable to get gpio base") | ||
|
||
|
||
def init_psu_gpio(self, pinnum): |
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.
this init_psu_gpio should move to your driver, we should not do init psu gpio on-the-fly.
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.
@jleveque, We will update init_psu_gpio after we updated our driver
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.
@pphuchar Please add this to our platform driver
@@ -53,7 +63,7 @@ def read_psu_statuses(self, pinnum): | |||
with open(gpio_file, 'r') as fd: | |||
retval = fd.read() | |||
except Exception as error: | |||
logging.error("Unable to open ", gpio_file, "file !") | |||
logging.error("Unable to open " + gpio_file + "file !") |
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 further raise in the plugin, retval = 'ERR' is not the convention to convene error to the caller.
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.
OK , I will add raise statement to report the error
@@ -53,7 +63,7 @@ def read_psu_statuses(self, pinnum): | |||
with open(gpio_file, 'r') as fd: | |||
retval = fd.read() | |||
except Exception as error: |
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 not catch general exception here, we should only catch exception we can handle in this case the io or file exception.
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.
OK, I will change it
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.
As per Guohan's comments above.
…get PSU status (sonic-net#1789)" This reverts commit 310c3f9.
f54b7d0b [Dynamic Buffer Calc][202012]Bug fix: Don't create lossless buffer profile for active ports without speed configured (sonic-net#1820) ac7f5cff Td2: Reclaim buffer from unused ports (sonic-net#1830) 04105a4b [debugcounterorch] check if counter type is supported before querying (sonic-net#1789) a67d8af6 [202012][portsorch] fix errors when moving port from one lag to another. (sonic-net#1819) Signed-off-by: Stephen Sun <stephens@nvidia.com>
* d240291 Update port_rates & rif_rates lua scripts to convert poll_interval to MS (sonic-net#1855) * a71a5d3 [acl mirror action] Mirror session ref count fix at acl rule attachment (sonic-net#1761) * 197f427 Fix vs test failure in test_buffer_traditional (sonic-net#1881) * 8471f42 Revert "[debugcounterorch] check if counter type is supported before querying… (sonic-net#1789)" (sonic-net#1884) Signed-off-by: Volodymyr Samotiy <volodymyrs@nvidia.com>
* d240291 Update port_rates & rif_rates lua scripts to convert poll_interval to MS (#1855) * a71a5d3 [acl mirror action] Mirror session ref count fix at acl rule attachment (#1761) * 197f427 Fix vs test failure in test_buffer_traditional (#1881) * 8471f42 Revert "[debugcounterorch] check if counter type is supported before querying… (#1789)" (#1884) Signed-off-by: Volodymyr Samotiy <volodymyrs@nvidia.com>
* d03ba4f [202012] [portstat, intfstat] added rates and utilization (sonic-net#1812) * 499ad3f [config reload] Fix config reload failure due to sonic.target job cancellation (sonic-net#1814) * 96d658c [202012][sonic installer] Add swap setup support (sonic-net#1815) * a9c6970 platform pre-check for reboot in 202012 branch (sonic-net#1788) * 0e0478b Unify the number format in the ourput of portstat and pfcstat in all cases (sonic-net#1795) * 2d1e00e [ecnconfig] Fix exception seen during display and add unit tests (sonic-net#1784) (sonic-net#1789) Signed-off-by: Volodymyr Samotiy <volodymyrs@nvidia.com>
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com> d03ba4fa [202012] [portstat, intfstat] added rates and utilization (sonic-net#1812) 499ad3f4 [config reload] Fix config reload failure due to sonic.target job cancellation (sonic-net#1814) 96d658c2 [202012][sonic installer] Add swap setup support (sonic-net#1815) a9c69702 platform pre-check for reboot in 202012 branch (sonic-net#1788) 0e0478ba Unify the number format in the ourput of portstat and pfcstat in all cases (sonic-net#1795) 2d1e00ed [ecnconfig] Fix exception seen during display and add unit tests (sonic-net#1784) (sonic-net#1789)
* d03ba4f [202012] [portstat, intfstat] added rates and utilization (#1812) * 499ad3f [config reload] Fix config reload failure due to sonic.target job cancellation (#1814) * 96d658c [202012][sonic installer] Add swap setup support (#1815) * a9c6970 platform pre-check for reboot in 202012 branch (#1788) * 0e0478b Unify the number format in the ourput of portstat and pfcstat in all cases (#1795) * 2d1e00e [ecnconfig] Fix exception seen during display and add unit tests (#1784) (#1789) Signed-off-by: Volodymyr Samotiy <volodymyrs@nvidia.com>
sonic-net#1789) * [debugcounterorch] check if counter type is supported before querying object availability count Signed-off-by: Stepan Blyschak <stepanb@nvidia.com>
- What I did
Fix bug that found on #1781
- How I did it
Update code to support dynamic sysfs gpio base.
- How to verify it
In SONiC run psuutil cli to test the following case:
PSU1: Absence
PSU1: Presence, no-power
PSU2: Absence
PSU1: Presence, no-power