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

[Mellanox] Credo Y-cable read_eeprom/write_eeprom API implementation #10320

Merged
merged 2 commits into from
Mar 30, 2022

Conversation

ayurkiv-nvda
Copy link
Contributor

@ayurkiv-nvda ayurkiv-nvda commented Mar 22, 2022

Why I did it

Need to implement read_eeprom/write_eeprom API for Credo Y-cable for Dual-Tor

How I did it

Used mlxreg utility for API implementation

How to verify it

run sfp unit tests

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

@lgtm-com
Copy link

lgtm-com bot commented Mar 22, 2022

This pull request introduces 1 alert when merging 2e9887e6f3263c87a7d754c27380ba0979f4bc6d into f8e1104 - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@ayurkiv-nvda ayurkiv-nvda force-pushed the credo_y_cable_azure branch 4 times, most recently from 3fa09c2 to ec5af34 Compare March 23, 2022 18:52
nazariig
nazariig previously approved these changes Mar 23, 2022
@nazariig
Copy link
Collaborator

@ayurkiv-nvda please fix the build:

==================================== ERRORS ====================================
______________________ ERROR collecting tests/test_sfp.py ______________________
ImportError while importing test module '/sonic/platform/mellanox/mlnx-platform-api/tests/test_sfp.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/usr/lib/python2.7/dist-packages/_pytest/python.py:450: in _importtestmodule
    mod = self.fspath.pyimport(ensuresyspath=importmode)
/usr/lib/python2.7/dist-packages/py/_path/local.py:668: in pyimport
    __import__(modname)
/usr/lib/python2.7/dist-packages/_pytest/assertion/rewrite.py:294: in load_module
    six.exec_(co, mod.__dict__)
/usr/lib/python2.7/dist-packages/six.py:709: in exec_
    exec("""exec _code_ in _globs_, _locs_""")
<string>:1: in <module>
    ???
tests/test_sfp.py:31: in <module>
    from .input_platform import output_sfp
E   ImportError: No module named input_platform

nazariig
nazariig previously approved these changes Mar 24, 2022
@liat-grozovik liat-grozovik changed the title Credo Y-cable read_eeprom/write_eeprom API implementation [Mellanox] Credo Y-cable read_eeprom/write_eeprom API implementation Mar 24, 2022
@ayurkiv-nvda
Copy link
Contributor Author

@ayurkiv-nvda please fix the build:

==================================== ERRORS ====================================
______________________ ERROR collecting tests/test_sfp.py ______________________
ImportError while importing test module '/sonic/platform/mellanox/mlnx-platform-api/tests/test_sfp.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/usr/lib/python2.7/dist-packages/_pytest/python.py:450: in _importtestmodule
    mod = self.fspath.pyimport(ensuresyspath=importmode)
/usr/lib/python2.7/dist-packages/py/_path/local.py:668: in pyimport
    __import__(modname)
/usr/lib/python2.7/dist-packages/_pytest/assertion/rewrite.py:294: in load_module
    six.exec_(co, mod.__dict__)
/usr/lib/python2.7/dist-packages/six.py:709: in exec_
    exec("""exec _code_ in _globs_, _locs_""")
<string>:1: in <module>
    ???
tests/test_sfp.py:31: in <module>
    from .input_platform import output_sfp
E   ImportError: No module named input_platform

Done

…entation

Signed-off-by: Andriy Yurkiv <ayurkiv@nvidia.com>
Signed-off-by: Andriy Yurkiv <ayurkiv@nvidia.com>
@ayurkiv-nvda
Copy link
Contributor Author

/azpw run Azure.sonic-buildimage

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-buildimage (Test kvmtest-t0)

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@@ -222,7 +305,11 @@ def get_presence(self):
return eeprom_raw is not None

# Read out any bytes from any offset
def read_eeprom(self, offset, num_bytes):
def _read_eeprom_specific_bytes(self, offset, num_bytes):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not critical, but I suggest renaming this function to _read_eeprom_sfp_bytes or a name you choose that tells what function is actually doing.

@liat-grozovik liat-grozovik merged commit 1e2e493 into sonic-net:master Mar 30, 2022
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.

6 participants