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

[sfpshow]: display eeprom data in table format #1030

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

Conversation

mudsut4ke
Copy link

@mudsut4ke mudsut4ke commented Aug 6, 2020

- What I did

- How I did it

  • Modify sfpshow script
  • Add table display option to show script

- How to verify it

- Previous command output (if the output of a command-line utility has changed)

admin@sonic:~$ show interfaces transceiver eeprom --help
Usage: show interfaces transceiver eeprom [OPTIONS] [INTERFACENAME]

  Show interface transceiver EEPROM information

Options:
  -d, --dom       Also display Digital Optical Monitoring (DOM) data
  --verbose       Enable verbose output
  -?, -h, --help  Show this message and exit.
admin@sonic:~$ 

- New command output (if the output of a command-line utility has changed)

admin@sonic:~$ show interfaces transceiver eeprom --help
Usage: show interfaces transceiver eeprom [OPTIONS] [INTERFACENAME]

  Show interface transceiver EEPROM information

Options:
  -d, --dom       Also display Digital Optical Monitoring (DOM) data
  -t, --table     Display SFP EEPROM data in table format
  --verbose       Enable verbose output
  -?, -h, --help  Show this message and exit.
admin@sonic:~$ 
admin@sonic:~$ show interfaces transceiver eeprom -t Ethernet8
Interface    Identifier       Connector    Vendor Name    Model Name          Serial Number    Nominal Bit Rate(100Mbs)
-----------  ---------------  -----------  -------------  ----------------  ---------------  --------------------------
Ethernet8    QSFP28 or later  LC           ColorChip ltd  C100QSFPCWDM400B         18130616                         255
admin@sonic:~$ show interfaces transceiver eeprom -t -d Ethernet8
Interface    Lane Number    Temp(C)                      Voltage(V)                   Current(mA)                  Tx Power(dBm)                      Rx Power(dBm)
-----------  -------------  ---------------------------  ---------------------------  ---------------------------  ---------------------------------  ----------------------------------
Ethernet8    Lane 1         32.1055 [low=0.0,high=58.0]  3.3769 [low=25.0,high=75.0]  56.762 [low=25.0,high=75.0]  0.4961 [low=-7.0006,high=2.2999]   0.4961 [low=-13.6957,high=2.2999]
             Lane 2         32.1055 [low=0.0,high=58.0]  3.3769 [low=25.0,high=75.0]  55.416 [low=25.0,high=75.0]  0.3782 [low=-7.0006,high=2.2999]   0.3782 [low=-13.6957,high=2.2999]
             Lane 3         32.1055 [low=0.0,high=58.0]  3.3769 [low=25.0,high=75.0]  41.614 [low=25.0,high=75.0]  0.5918 [low=-7.0006,high=2.2999]   0.5918 [low=-13.6957,high=2.2999]
             Lane 4         32.1055 [low=0.0,high=58.0]  3.3769 [low=25.0,high=75.0]  41.648 [low=25.0,high=75.0]  -0.1909 [low=-7.0006,high=2.2999]  -0.1909 [low=-13.6957,high=2.2999]
admin@sonic:~$ 

Signed-off-by: Wirut Getbamrung [wgetbumr@celestica.com]

@lgtm-com

This comment has been minimized.

@lguohan
Copy link
Contributor

lguohan commented Aug 8, 2020

retest this please

@mudsut4ke mudsut4ke marked this pull request as ready for review August 11, 2020 06:08
@jleveque jleveque requested a review from keboliu August 11, 2020 23:14
@jleveque
Copy link
Contributor

Retest this please

@lguohan
Copy link
Contributor

lguohan commented Aug 12, 2020

can you resolve the conflict?

@lguohan
Copy link
Contributor

lguohan commented Aug 15, 2020

can you also please add the unit test?

@mudsut4ke
Copy link
Author

@lguohan

can you resolve the conflict?

  • resolved

can you also please add the unit test?

@mudsut4ke
Copy link
Author

can you also please add the unit test?

@lguohan , added in 72f0f60

@jleveque
Copy link
Contributor

jleveque commented Sep 4, 2020

LGTM. @keboliu to review, as well.

'Nominal Bit Rate(100Mbs)']

table_header_dom = ['Interface', 'Lane Number',
'Temp(C)', 'Voltage(V)',
Copy link
Collaborator

@keboliu keboliu Sep 9, 2020

Choose a reason for hiding this comment

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

I have a concern about the way you handle the temperature, it's not some sensor that monitored per lane, here you added it to each lane, it's kind of misleading. The same comment goes to the voltage.

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.

4 participants