-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
DellEMC : Platform2.0 API Implementation for Chassis [S6000, S6100, Z9100], Fan [S6000] #3381
DellEMC : Platform2.0 API Implementation for Chassis [S6000, S6100, Z9100], Fan [S6000] #3381
Conversation
@ArunSaravananBalachandran Can you please resolve the conflicts ? PR looks good to me but I want to try on some of the devices to check the changes. |
@sujinmkang Merge conflicts have been resolved. |
@@ -366,13 +408,11 @@ def install_component_firmware(self, component_name, image_path): | |||
elif component_name == self._component_name_list[1]: # CPLD1 |
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.
Please correct the comment to CPLD not CPLD1
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.
Have addressed the review comment.
return self.STATUS_LED_COLOR_GREEN | ||
else: | ||
return self.STATUS_LED_COLOR_OFF | ||
|
||
def get_target_speed(self): |
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 indentation is wrong here. It will break the reboot cause determination other cli commands which are related to sonic_platform.
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 actually fixed in #3363 (your another PR)
Which one do you want to get in first?
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.
3363 is merged so you got the conflicts on this PR. please resolve the conflicts.
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.
Latest Merge conflicts have been resolved.
- What I did
- How I did it
- How to verify it
Wrote python script to load Chassis class and then call the APIs accordingly.
UT Logs - UT_logs-S6000.txt, UT_logs-S6100.txt, UT_logs-Z9100.txt
- Description for the changelog
DellEMC : Platform2.0 API Implementation for Chassis [S6000, S6100, Z9100], Fan [S6000]
- A picture of a cute animal (not mandatory but encouraged)