Add get_transceiver_status and get_transceiver_pm to API interface #315

longhuan-cisco · 2022-10-18T00:35:50Z

Add get_transceiver_status and get_transceiver_pm to API interface

Description

Add get_transceiver_status to API interface
Add get_transceiver_pm API interface in xcvr_api.py
Ignore if "Rx Signal Power" related fields not existing.
Fix a typo in get_pm_all()

Motivation and Context

How Has This Been Tested?

Plz refer to parent PR for UT.

Additional Information (Optional)

Note
This PR serves as dependency for sonic-net/sonic-platform-daemons#304

linux-foundation-easycla · 2022-10-18T00:35:53Z

The committers listed above are authorized under a signed CLA.

✅ login: longhuan-cisco (5752d7e, 0736d15)

longhuan-cisco · 2022-10-19T23:03:36Z

@prgeor please review. This is dependency of sonic-net/sonic-platform-daemons#304

mihirpat1

I think you will also need to modify testing script to improve code coverage since it's less than 80% currently.

mihirpat1 · 2022-10-26T00:01:47Z

sonic_platform_base/sonic_xcvr/api/public/c_cmis.py

-        trans_status['rxsigpowerhighwarning_flag'] = self.vdm_dict['Rx Signal Power [dBm]'][1][7]
-        trans_status['rxsigpowerlowwarning_flag'] = self.vdm_dict['Rx Signal Power [dBm]'][1][8]
+        try:
+            trans_status['rxsigpowerhighalarm_flag'] = self.vdm_dict['Rx Signal Power [dBm]'][1][5]


Just curious - Are we seeing KeyError exception for ZR since I didn't find this being populated in the UT.

yes.
And I saw similar key error handling in c_cmis.py:get_transceiver_threshold_info() for self.vdm_dict['Rx Signal Power [dBm]']

longhuan-cisco · 2022-10-27T07:49:07Z

I think you will also need to modify testing script to improve code coverage since it's less than 80% currently.

Added testcase in test_ccmis.py to cover the case of 'Rx Signal Power [dBm]' not present.

longhuan-cisco · 2022-11-03T03:02:00Z

@prgeor @mihirpat1 I took care of the existing comments, can you please review?

Add get_transceiver_pm API interface

longhuan-cisco · 2022-11-08T05:24:19Z

Added pm table related change, to use this PR to track changes for both status and pm.
@prgeor @mihirpat1 Could you please take another look?

prgeor · 2022-11-22T02:24:16Z

sonic_platform_base/sonic_xcvr/api/public/c_cmis.py

+            trans_status['rxsigpowerlowalarm_flag'] = self.vdm_dict['Rx Signal Power [dBm]'][1][6]
+            trans_status['rxsigpowerhighwarning_flag'] = self.vdm_dict['Rx Signal Power [dBm]'][1][7]
+            trans_status['rxsigpowerlowwarning_flag'] = self.vdm_dict['Rx Signal Power [dBm]'][1][8]
+        except KeyError:


@longhuan-cisco how do we know if there is a key error?

Added a debug log here in the case of key error.

prgeor · 2022-11-22T02:25:07Z

sonic_platform_base/sonic_xcvr/api/xcvr_api.py

@@ -133,6 +133,23 @@ def get_transceiver_threshold_info(self):
        """
        raise NotImplementedError

+    def get_transceiver_status(self):
+        """
+        Retrieves transceiver status of this SFP


can you provide more details in the API description what does status mean

sure, added.

we have an existing API "get_transceiver_bulk_status", should we consider combining the new function with this existing one? Otherwise, I would suggest reconsidering the name of the new API for differentiation.

prgeor · 2022-11-22T02:25:32Z

sonic_platform_base/sonic_xcvr/api/xcvr_api.py

+
+    def get_transceiver_pm(self):
+        """
+        Retrieves PM for this xcvr


could you add more details in this API what does PM mean and where its applicable?

sure, added.

lgtm-com · 2022-11-23T10:49:13Z

This pull request introduces 1 alert when merging 5261da7 into ce9aacb - view on LGTM.com

new alerts:

1 for Unnecessary pass

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog.

keboliu · 2022-11-24T06:33:28Z

sonic_platform_base/sonic_xcvr/api/public/c_cmis.py

+            trans_status['rxsigpowerhighwarning_flag'] = self.vdm_dict['Rx Signal Power [dBm]'][1][7]
+            trans_status['rxsigpowerlowwarning_flag'] = self.vdm_dict['Rx Signal Power [dBm]'][1][8]
+        except KeyError:
+            helper_logger.log_debug('Rx Signal Power [dBm] not present in VDM')


will the 'debug' level log print out in the running time?

Can we print more generic key error that can print any Key error name?

prgeor · 2022-11-28T20:42:21Z

sonic_platform_base/sonic_xcvr/api/xcvr_api.py

+        cfo_avg                      = FLOAT                            ; carrier frequency offset avg
+        cfo_min                      = FLOAT                            ; carrier frequency offset min
+        cfo_max                      = FLOAT                            ; carrier frequency offset max
+        soproc_avg                   = FLOAT                            ; state of polarization rate of change avg


@longhuan-cisco 'evm_min/avg/max' is missing

…onic-net#315) * Add get_transceiver_status to API interface * Add test_ccmis support * Add get_transceiver_pm API interface * Add debug log and more description for interface * Remove unnecessary pass

keboliu · 2023-01-17T11:16:59Z

PR sonic-net/sonic-platform-daemons#304 has a dependency on this PR, it has already been included in 202211, this PR shall also be cherry-picked to 202211, otherwise, 202211 will be broken. @prgeor @longhuan-cisco @StormLiangMS

) * Add get_transceiver_status to API interface * Add test_ccmis support * Add get_transceiver_pm API interface * Add debug log and more description for interface * Remove unnecessary pass

…t#315)

Add get_transceiver_status to API interface

5752d7e

longhuan-cisco mentioned this pull request Oct 18, 2022

Add new fields to status/dom_sensor/pm tables in STATE_DB for CMIS/C-CMIS sonic-net/sonic-platform-daemons#304

Merged

mihirpat1 reviewed Oct 26, 2022

View reviewed changes

Add test_ccmis support

0736d15

Add get_transceiver_pm API interface

35170fd

longhuan-cisco requested a review from mihirpat1 October 31, 2022 22:32

Merge pull request #2 from longhuan-cisco/add_pm_tbl_dep

542e67d

Add get_transceiver_pm API interface

longhuan-cisco changed the title ~~Add get_transceiver_status to API interface~~ Add get_transceiver_status and get_transceiver_pm to API interface Nov 8, 2022

prgeor reviewed Nov 22, 2022

View reviewed changes

prgeor self-assigned this Nov 22, 2022

prgeor added the CMIS label Nov 22, 2022

prgeor requested a review from keboliu November 22, 2022 02:35

mihirpat1 previously approved these changes Nov 22, 2022

View reviewed changes

Add debug log and more description for interface

5261da7

longhuan-cisco dismissed mihirpat1’s stale review via 5261da7 November 23, 2022 10:26

longhuan-cisco requested review from prgeor and removed request for keboliu November 23, 2022 10:39

Remove unnecessary pass

0369b0c

keboliu reviewed Nov 24, 2022

View reviewed changes

prgeor reviewed Nov 28, 2022

View reviewed changes

prgeor approved these changes Dec 6, 2022

View reviewed changes

prgeor merged commit 8f2dffb into sonic-net:master Dec 6, 2022

mihirpat1 added the Request for 202205 Branch label Jan 15, 2023

keboliu added the Request for 202211 Branch label Jan 17, 2023

yxieca added the Included in 202205 Branch label Jan 19, 2023

StormLiangMS added the Included in 202211 Branch label Jan 23, 2023

oleksandrivantsiv pushed a commit to oleksandrivantsiv/sonic-platform-common that referenced this pull request Oct 25, 2024

[thermalctld] fix some redundant removal of state DB tables (sonic-ne…

9657a26

…t#315)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add get_transceiver_status and get_transceiver_pm to API interface #315

Add get_transceiver_status and get_transceiver_pm to API interface #315

longhuan-cisco commented Oct 18, 2022 •

edited

Loading

linux-foundation-easycla bot commented Oct 18, 2022 •

edited

Loading

longhuan-cisco commented Oct 19, 2022

mihirpat1 left a comment

mihirpat1 Oct 26, 2022

longhuan-cisco Oct 27, 2022

longhuan-cisco commented Oct 27, 2022

longhuan-cisco commented Nov 3, 2022

longhuan-cisco commented Nov 8, 2022 •

edited

Loading

prgeor Nov 22, 2022

longhuan-cisco Nov 23, 2022

prgeor Nov 22, 2022

longhuan-cisco Nov 23, 2022

keboliu Nov 24, 2022

prgeor Nov 22, 2022

longhuan-cisco Nov 23, 2022

lgtm-com bot commented Nov 23, 2022

keboliu Nov 24, 2022

prgeor Nov 28, 2022

prgeor Nov 28, 2022

keboliu commented Jan 17, 2023 •

edited

Loading

Add get_transceiver_status and get_transceiver_pm to API interface #315

Add get_transceiver_status and get_transceiver_pm to API interface #315

Conversation

longhuan-cisco commented Oct 18, 2022 • edited Loading

Description

Motivation and Context

How Has This Been Tested?

Additional Information (Optional)

linux-foundation-easycla bot commented Oct 18, 2022 • edited Loading

longhuan-cisco commented Oct 19, 2022

mihirpat1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

longhuan-cisco commented Oct 27, 2022

longhuan-cisco commented Nov 3, 2022

longhuan-cisco commented Nov 8, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lgtm-com bot commented Nov 23, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keboliu commented Jan 17, 2023 • edited Loading

longhuan-cisco commented Oct 18, 2022 •

edited

Loading

linux-foundation-easycla bot commented Oct 18, 2022 •

edited

Loading

longhuan-cisco commented Nov 8, 2022 •

edited

Loading

keboliu commented Jan 17, 2023 •

edited

Loading