sonic_sfp: Support for DOM Threshold values for EEPROM dump #33
Conversation
sonic_sfp/sff8436.py
Outdated
| @@ -1229,3 +1229,472 @@ def get_data(self): | |||
|
|
|||
| def get_data_pretty(self): | |||
| return sffbase.get_data_pretty(self, self.dom_data) | |||
|
|
|||
| class sff8436DomThreshold(sffbase): | |||
There was a problem hiding this comment.
I would suggest adding new APIs to current sff8436Dom class instead of having this new class.
The benefit I can see could be:
-
reduce the duplicated code, there is no reason to have two copies of 'cal_temperature', 'calc_voltage', etc.
-
The threshold is part of the DOM, add new APIs would be more intuitional.
There was a problem hiding this comment.
Hi,
Addressed the review comment. Removed the duplicate code and added new APIs in sff8436Dom
sonic_sfp/sfputilbase.py
Outdated
| @@ -16,6 +16,7 @@ | |||
| from .sff8472 import sff8472Dom # Dot module supports both Python 2 and Python 3 using explicit relative import methods | |||
| from .sff8436 import sff8436InterfaceId # Dot module supports both Python 2 and Python 3 using explicit relative import methods | |||
| from .sff8436 import sff8436Dom # Dot module supports both Python 2 and Python 3 using explicit relative import methods | |||
| from .sff8436 import sff8436DomThreshold # Dot module supports both Python 2 and Python 3 using explicit relative import methods | |||
There was a problem hiding this comment.
To provide a complete solution, I would suggest implementing the same set of APIs also for sff8472.
There was a problem hiding this comment.
Hi,
Implemented the same solution for sff8472
|
@keboliu: Please review new changes. |
sonic_sfp/sfputilbase.py
Outdated
| @@ -923,6 +946,10 @@ def get_transceiver_dom_info_dict(self, port_num): | |||
| if sfpd_obj is None: | |||
| return None | |||
|
|
|||
| sfpdth_obj = sff8436DomThreshold() | |||
There was a problem hiding this comment.
where is this sff8436DomThreshold() defined? shouldn't it been removed?
There was a problem hiding this comment.
Hi @keboliu
Removed the irrelevant code
|
Hi @keboliu |
sonic_sfp/sfputilbase.py
Outdated
| @@ -1005,6 +1052,24 @@ def get_transceiver_dom_info_dict(self, port_num): | |||
| transceiver_dom_info_dict['tx3bias'] = dom_channel_monitor_data['data']['TX3Bias']['value'] | |||
| transceiver_dom_info_dict['tx4bias'] = dom_channel_monitor_data['data']['TX4Bias']['value'] | |||
|
|
|||
| # Threshold Data | |||
| transceiver_dom_info_dict['temphighalarm'] = dom_module_threshold_data['data']['TempHighAlarm']['value'] | |||
There was a problem hiding this comment.
are you able to decouple the threshold reading and the voltage/temp/bias reading? since the threshold is constant, we don't want to read them in every iteration for voltage/power/bias reading. This will introduce unnecessary load to CPU since I2C access is time-consuming.
There was a problem hiding this comment.
Hi,
I have decoupled it correctly this time. Implemented a new function get_transceiver_dom_threshold_info_dict.
There was a problem hiding this comment.
This change looks good to me.
One more thing is that in folder https://github.com/Azure/sonic-platform-common/tree/master/sonic_platform_base/sonic_sfp we have copies of all the files in https://github.com/Azure/sonic-platform-common/tree/master/sonic_sfp
The intention to keep this copy is that in the near future(could be next release), we will only keep files under ../sonic_platform_base/sonic_sfp and others will be removed along with plugins.
so please also merge your change to the files under ../sonic_platform_base/sonic_sfp
There was a problem hiding this comment.
Hi,
Ported my changes from sonic_sfp to sonic_platform_base/sonic_sfp
This change is added to dump ChannelMonitor Threshold and Module Monitor Threshold values as part of DOM EEPROM dump output.
Added code change in sff8436.py,sfputilshow.py, xcvrd, sfpshow .. etc
Verified the output in following optics
a) AVAGO
b) Finisar
c) DELL
Seeing the following issue of Values not getting dumped properly from day 1
Some channel monitor values are not displayed correctly.
Will raise a seperate issue for that.
Ethernet124: SFP EEPROM detected
Connector: MPOx12
Encoding: 64B66B
Extended Identifier: Power Class 1(1.5W max)
Extended RateSelect Compliance: QSFP+ Rate Select Version 1
Identifier: QSFP+ or later
Length OM3(2m): 50
Nominal Bit Rate(100Mbs): 103
Specification compliance:
10/40G Ethernet Compliance Code: 40GBASE-SR4
Vendor Date Code(YYYY-MM-DD Lot): 2010-09-01
Vendor Name: AVAGO
Vendor OUI: 00-17-6a
Vendor PN: AFBR-79E4Z-D
Vendor Rev: 01
Vendor SN: QA340092
ChannelMonitorValues:
RX1Power: -16.3827dBm
RX2Power: -infdBm <<<<<<<<<<<<<<<<<<<<<<
RX3Power: -15.5284dBm
RX4Power: -infdBm <<<<<<<<<<<<<<<<<<<<<<<
TX1Bias: 5.9720mA
TX2Bias: 5.3740mA
TX3Bias: 5.3800mA
TX4Bias: 6.0540mA