Skip to content
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

[Mellanox] Initialize system LED color to green for 201911 #4743

Merged
merged 2 commits into from
Jun 16, 2020

Conversation

Junchao-Mellanox
Copy link
Collaborator

@Junchao-Mellanox Junchao-Mellanox commented Jun 10, 2020

- Why I did it

Before adding the system led control which is planned for 202006, set the system LED color to green by default.

- How I did it

Initialize system status LED color to green in the constructor of chassis.py.

- How to verify it

Manual test on SN2700, SN3800 and SN4700

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@lgtm-com
Copy link

lgtm-com bot commented Jun 10, 2020

This pull request introduces 1 alert when merging 4a85792 into 0a70571 - view on LGTM.com

new alerts:

  • 1 for Except block handles 'BaseException'

@Junchao-Mellanox
Copy link
Collaborator Author

This pull request introduces 1 alert when merging 4a85792 into 0a70571 - view on LGTM.com

new alerts:

  • 1 for Except block handles 'BaseException'

We need to handle all exception here to avoid this function break chassis initialization.

Copy link

@henry-ma henry-ma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest adding some comments like before adding the system led control which is planned for 202006, set the system led to green as default.

@Junchao-Mellanox
Copy link
Collaborator Author

I suggest adding some comments like before adding the system led control which is planned for 202006, set the system led to green as default.

Sure.

@liat-grozovik
Copy link
Collaborator

Why not having that on master and once the system led will be fully available?

@Junchao-Mellanox
Copy link
Collaborator Author

Why not having that on master and once the system led will be fully available?

On master, system led will depends on Led class which is a part of PSU/Fan LED feature. Since PSU/Fan LED feature will not be included in 201911, we can't reuse that part in 201911.

This is for 201911 branch only
"""
try:
with open(LED_GREEN, 'w') as f:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Junchao-Mellanox maybe it is better to have more meaningful name, e.g. SYSTEM_STATUS_LED. This will help to differentiate from other board leds

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@lgtm-com
Copy link

lgtm-com bot commented Jun 15, 2020

This pull request introduces 1 alert when merging ff7c141 into 093d773 - view on LGTM.com

new alerts:

  • 1 for Except block handles 'BaseException'

@liat-grozovik liat-grozovik merged commit 62690f5 into sonic-net:201911 Jun 16, 2020
@Junchao-Mellanox Junchao-Mellanox deleted the init-system-led branch September 18, 2020 05:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants