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

Syslog for transceiver high/low temperature alarm #465

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

chiourung
Copy link
Contributor

@chiourung chiourung commented Apr 11, 2024

Add syslog for high/low temperature alarm/warning(compare the temperature with threshold)

Description

Add syslog for high/low temperature alarm/warning(compare the temperature with threshold)

Motivation and Context

The ZR transceiver has a protective mechanism that shuts down the transceiver when the temperature is high.
However, the user is not aware when the ZR shuts down due to high temperature.
Add syslog for this situation.

How Has This Been Tested?

syslog:

NOTICE pmon#xcvrd: Ethernet504: temperature status change from normal to temperature high warning

NOTICE pmon#xcvrd: Ethernet504: temperature status change from temperature high warning to temperature high alarm

NOTICE pmon#xcvrd: Ethernet504: temperature status change from temperature high alarm to normal

NOTICE pmon#xcvrd: Ethernet480: temperature status change from normal to temperature high warning

NOTICE pmon#xcvrd: Ethernet488: temperature status change from normal to temperature high warning

NOTICE pmon#xcvrd: Ethernet480: temperature status change from temperature high warning to temperature high alarm

NOTICE pmon#xcvrd: Ethernet488: temperature status change from temperature high warning to normal

NOTICE pmon#xcvrd: Ethernet480: temperature status change from temperature high alarm to normal

Additional Information (Optional)

Add syslog for high/low temperature alarm/warning(compare the temperature with threshold)

Signed-off-by: chiourung_huang <chiourung_huang@edge-core.com>
Copy link
Collaborator

@prgeor prgeor left a comment

Choose a reason for hiding this comment

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

@mihirpat1 can you review

@prgeor
Copy link
Collaborator

prgeor commented Apr 21, 2024

@chiourung please fix teh build failrue

@prgeor
Copy link
Collaborator

prgeor commented Apr 24, 2024

@chiourung Please update these secions

Description
Motivation and Contex

@prgeor prgeor requested a review from mihirpat1 June 20, 2024 21:00
}

for physical_port, physical_port_name in get_physical_port_name_dict(logical_port_name, self.port_mapping).items():
ori_temp_status = temperature_status.get(physical_port)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please rename this to orig_temp_status?

templowwarning = dom_th_info_dict.get("templowwarning")
if temperature != 'N/A' and temphighalarm != 'N/A' and templowalarm != 'N/A' and \
temphighwarning != 'N/A' and templowwarning != 'N/A':
if float(temperature) > float(temphighalarm):
Copy link
Contributor

Choose a reason for hiding this comment

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

@chiourung Can you please update the PR description with the snippet from the C_CMIS spec showing the shutdown behavior of the module due to high temperature.
Also, module shuts down only if current module temperature > temphighalarm or temphighwarning?

templowwarning = dom_th_info_dict.get("templowwarning")
if temperature != 'N/A' and temphighalarm != 'N/A' and templowalarm != 'N/A' and \
temphighwarning != 'N/A' and templowwarning != 'N/A':
if float(temperature) > float(temphighalarm):
Copy link
Contributor

Choose a reason for hiding this comment

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

@chiourung Does the module not shutdown if temperature) == temphighalarm?

TEMP_LOW_WARNING = 4

TEMP_ERROR_TO_DESCRIPTION_DICT = {
TEMP_NORMAL: "normal",
Copy link
Contributor

Choose a reason for hiding this comment

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

@chiourung

Suggested change
TEMP_NORMAL: "normal",
TEMP_NORMAL: "temperature normal",


if ori_temp_status != new_temp_status:
temperature_status[physical_port] = new_temp_status
helper_logger.log_notice("{}: temperature status change from {} to {}".format(
Copy link
Contributor

Choose a reason for hiding this comment

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

@chiourung

Suggested change
helper_logger.log_notice("{}: temperature status change from {} to {}".format(
helper_logger.log_notice("{}: temperature status changed from {} to {}".format(

Comment on lines +1801 to +1809
new_temp_status = TEMP_HIGH_ALARM
elif float(temperature) > float(temphighwarning):
new_temp_status = TEMP_HIGH_WARNING
elif float(temperature) < float(templowalarm):
new_temp_status = TEMP_LOW_ALARM
elif float(temperature) < float(templowwarning):
new_temp_status = TEMP_LOW_WARNING
else:
new_temp_status = TEMP_NORMAL
Copy link
Collaborator

Choose a reason for hiding this comment

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

@chiourung Instead of Xcvrd doing the comparison, there are module flags for low/high warning thresholds in byte 9. Why not use those latched flag?

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@prgeor These flags are COR (Clear on Read). Cannot be used to determine if the temperature is from alarm/warning to normal. The comparison of the temperature can be kept to record when the temperature is alarm / warning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants