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

[Y-Cable][Credo] Credo implementation of YCable class which inherits from YCableBase required for Y-Cable API's in sonic-platform-daemons #203

Merged
merged 33 commits into from
Aug 6, 2021

Conversation

vdahiya12
Copy link
Contributor

@vdahiya12 vdahiya12 commented Jun 28, 2021

This PR adds support for YCable class required for platform-daemons to use the YCable API's for credo.

Description

Basically a vendor specific implementation of abstract YCableBase class .
detailed design discussion can be found https:/Azure/SONiC/pull/757/files

Motivation and Context

Required for transitioning to vendor agnostic API's to be called by xcvrd, so that all types of cables can be called.

How Has This Been Tested?

Ran the changes on Arista7050cx3 switch, making changes inside the container.

Additional Information (Optional)

Signed-off-by: vaibhav-dahiya [email protected]

inherits from YCableBase required for Y-Cable API's in sonic-platform-daemons

Signed-off-by: vaibhav-dahiya <[email protected]>
@lgtm-com
Copy link

lgtm-com bot commented Jun 28, 2021

This pull request introduces 13 alerts when merging e7cb18a into 4533f82 - view on LGTM.com

new alerts:

  • 4 for Unused local variable
  • 4 for Variable defined multiple times
  • 2 for Unreachable code
  • 1 for Testing equality to None
  • 1 for Except block handles 'BaseException'
  • 1 for Unused import

xinyulin and others added 2 commits June 29, 2021 22:45
    4 for Unused local variable
    4 for Variable defined multiple times
    2 for Unreachable code
    1 for Testing equality to None
    1 for Except block handles 'BaseException'
    1 for Unused import

Signed-off-by: xinyu <[email protected]>
Signed-off-by: vaibhav-dahiya <[email protected]>
@lgtm-com
Copy link

lgtm-com bot commented Jun 30, 2021

This pull request introduces 12 alerts when merging 482da93 into 4533f82 - view on LGTM.com

new alerts:

  • 4 for Unused local variable
  • 4 for Variable defined multiple times
  • 2 for Unreachable code
  • 1 for Testing equality to None
  • 1 for Except block handles 'BaseException'

Signed-off-by: vaibhav-dahiya <[email protected]>
@lgtm-com
Copy link

lgtm-com bot commented Jun 30, 2021

This pull request introduces 1 alert when merging 1962fc2 into 4533f82 - view on LGTM.com

new alerts:

  • 1 for Syntax error

@lgtm-com
Copy link

lgtm-com bot commented Jul 3, 2021

This pull request introduces 1 alert when merging a3d976a into 4533f82 - view on LGTM.com

new alerts:

  • 1 for Variable defined multiple times

@lgtm-com
Copy link

lgtm-com bot commented Jul 6, 2021

This pull request introduces 1 alert when merging e2834d1 into 4533f82 - view on LGTM.com

new alerts:

  • 1 for Unreachable code

@lgtm-com
Copy link

lgtm-com bot commented Jul 12, 2021

This pull request introduces 1 alert when merging 635dc4f into 4533f82 - view on LGTM.com

new alerts:

  • 1 for Variable defined multiple times

@@ -0,0 +1,13 @@
mapping = {
"Credo": {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we please make the vendor name and model names in lower cases, to comply with fetching vendor mapping attribute.
example
mapping = {
"credo": {
"cacl2x321p2pa1ms": "credo.y_cable_credo"
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, we can convert the PN and vendor name in lower case.

lguohan
lguohan previously approved these changes Jul 26, 2021
Signed-off-by: vaibhav-dahiya <[email protected]>
vdahiya12 and others added 2 commits July 29, 2021 02:03
lguohan
lguohan previously approved these changes Aug 2, 2021
@vdahiya12 vdahiya12 merged commit cd3cca7 into sonic-net:master Aug 6, 2021
@vdahiya12
Copy link
Contributor Author

Hi @qiluo-msft , please cherry pick this to 202012, so that I can update the submodule.

qiluo-msft pushed a commit that referenced this pull request Aug 12, 2021
…from YCableBase required for Y-Cable API's in sonic-platform-daemons (#203)

This PR adds support for YCable class required for platform-daemons to use the YCable API's for credo.

Description
Basically a vendor specific implementation of abstract YCableBase class .
detailed design discussion can be found https:/Azure/SONiC/pull/757/files

Motivation and Context
Required for transitioning to vendor agnostic API's to be called by xcvrd, so that all types of cables can be called.

How Has This Been Tested?
Ran the changes on Arista7050cx3 switch, making changes inside the container.

Co-authored-by: xinyu <[email protected]>
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.

5 participants