[sfp] Fix issue: Application Advertisement is not well formatted #2491
Conversation
| @@ -38,25 +40,6 @@ from utilities_common import multi_asic as multi_asic_util | |||
| from utilities_common.platform_sfputil_helper import is_rj45_port, RJ45_PORT_TYPE | |||
|
|
|||
| # TODO: We should share these maps and the formatting functions between sfputil and sfpshow | |||
There was a problem hiding this comment.
is this TODO is still valid? what is missing to close it?
There was a problem hiding this comment.
Yes, it is still valid. Now show interface transceiver eeprom and sfputil show eeprom have very similar output. However, these two commands have their own implementation. I suppose they shall share the same implementation, but I did not do it in this PR because it would be a big change.
| @@ -45,25 +47,6 @@ | |||
| MAX_LPL_FIRMWARE_BLOCK_SIZE = 116 #Bytes | |||
|
|
|||
| # TODO: We should share these maps and the formatting functions between sfputil and sfpshow | |||
|
LGTM beside of the comment which is more like a question than a suggestion |
|
/azp run Azure.sonic-utilities |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| @@ -45,25 +47,6 @@ | |||
| MAX_LPL_FIRMWARE_BLOCK_SIZE = 116 #Bytes | |||
|
|
|||
| # TODO: We should share these maps and the formatting functions between sfputil and sfpshow | |||
| QSFP_DATA_MAP = { | |||
There was a problem hiding this comment.
this is still used by QSFP28/and QSFP+
There was a problem hiding this comment.
I did not remove it. I just move it to a common place. See sfp_helper.py.
|
@Junchao-Mellanox can you add lane assignment mask also in hex? |
|
This pull request introduces 2 alerts when merging 52d6c81 into a5eb26b - view on LGTM.com new alerts:
|
|
Hi prgeor, would you please review again? |
| @@ -0,0 +1,53 @@ | |||
| import ast | |||
|
|
|||
| QSFP_DATA_MAP = { | |||
There was a problem hiding this comment.
why are we then keeping QSFP_DD_DATA_MAP {} in sfputil? can we move that also here?
There was a problem hiding this comment.
Hi @prgeor , QSFP_DD_DATA_MAP is not related to my change. This PR is aimed to fix the format of Application Advertisement. Moving QSFP_DD_DATA_MAP is part of refactoring sfputi/main.py and sfpshow, I suppose it is out of the scope of this PR.
Moving similar logic of sfputil/main.py and sfpshow to a common place is a good idea, we shall do it in another PR.
|
@Junchao-Mellanox can you check the build failure |
|
/azpw run Azure.sonic-utilities |
|
/AzurePipelines run Azure.sonic-utilities |
|
LGTM check is stuck. Will override. |
|
Hi @yxieca , could you please help cherry-pick to 202205? |
…ic-net#2491) #### What I did Currently, the value of "Application Advertisement" is a python dict to string which is not well formatted. This PR re-format the output of "Application Advertisement" to a human readable string #### How I did it Set the output to similar to 202012 #### How to verify it Unit test Manual test #### Previous command output (if the output of a command-line utility has changed) ``` Ethernet200: SFP EEPROM detected Application Advertisement: {1: {'host_electrical_interface_id': '400GAUI-8 C2M (Annex 120E)', 'module_media_interface_id': '400GBASE-DR4 (Cl 124)', 'media_lane_count': 4, 'host_lane_count': 8, 'host_lane_assignment_options': 1}, 2: {'host_electrical_interface_id': '100GAUI-2 C2M (Annex 135G)', 'module_media_interface_id': '100GBASE-DR (Cl 140)', 'media_lane_count': 1, 'host_lane_count': 2, 'host_lane_assignment_options': 85}} ... ``` #### New command output (if the output of a command-line utility has changed) ``` Ethernet200: SFP EEPROM detected Application Advertisement: 400GAUI-8 C2M (Annex 120E) - 400GBASE-DR4 (Cl 124) - 0x1 100GAUI-2 C2M (Annex 135G) - 100GBASE-DR (Cl 140) - 0x55 ... ```
#### What I did
Currently, the value of "Application Advertisement" is a python dict to string which is not well formatted.
This PR re-format the output of "Application Advertisement" to a human readable string
#### How I did it
Set the output to similar to 202012
#### How to verify it
Unit test
Manual test
#### Previous command output (if the output of a command-line utility has changed)
```
Ethernet200: SFP EEPROM detected
Application Advertisement: {1: {'host_electrical_interface_id': '400GAUI-8 C2M (Annex 120E)', 'module_media_interface_id': '400GBASE-DR4 (Cl 124)', 'media_lane_count': 4, 'host_lane_count': 8, 'host_lane_assignment_options': 1}, 2: {'host_electrical_interface_id': '100GAUI-2 C2M (Annex 135G)', 'module_media_interface_id': '100GBASE-DR (Cl 140)', 'media_lane_count': 1, 'host_lane_count': 2, 'host_lane_assignment_options': 85}}
...
```
#### New command output (if the output of a command-line utility has changed)
```
Ethernet200: SFP EEPROM detected
Application Advertisement: 400GAUI-8 C2M (Annex 120E) - 400GBASE-DR4 (Cl 124) - 0x1
100GAUI-2 C2M (Annex 135G) - 100GBASE-DR (Cl 140) - 0x55
...
```
What I did
Currently, the value of "Application Advertisement" is a python dict to string which is not well formatted.
This PR re-format the output of "Application Advertisement" to a human readable string
How I did it
Set the output to similar to 202012
How to verify it
Unit test
Manual test
Previous command output (if the output of a command-line utility has changed)
New command output (if the output of a command-line utility has changed)