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

Support RJ45 port #37

Conversation

stephenxs
Copy link
Collaborator

  1. A derived class representing RJ45 ports is introduced. get_presence always returns True
  2. SFP error event is leveraged to represent "unknown" and "not present" states.
    By doing so, most of code change for supporting RJ45 is restricted in platform API and CLI
    xcvrd won't need to be updated for this

Signed-off-by: Stephen Sun [email protected]

Why I did it

How I did it

How to verify it

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

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106

Description for the changelog

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

1. A derived class representing RJ45 ports is introduced. get_presence always returns True
2. SFP error event is leveraged to represent "unknown" and "not present" states.
   By doing so, most of code change for supporting RJ45 is restricted in platform API and CLI
   xcvrd won't need to be updated for this

Signed-off-by: Stephen Sun <[email protected]>
# check if it's unknown event
if event == '0': # Remove event
unplug_port.add(index)
elif event != '1':
Copy link
Owner

Choose a reason for hiding this comment

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

will the "unkown" status also reported from PMPE or need to get it by calling "sx_api_port_status"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It should be. Otherwise there is inconvenience between PMPE and PMAOS, which IMO is a bug. Anyway, let’s double check.
The only exception AFAIK is the initialization flow. I don’t know whether the status change occurring before registered to the PMPE channel will be reported after registered. If not, we need to fetch the statuses for all the ports and report events for the ports that are unknown or not present when get_change_event is called for the first time.

Copy link
Owner

Choose a reason for hiding this comment

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

let's perform some tests for this after the holiday and I am merging your change for now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Currently, there is a bug in sdk. No matter which port is unplugged, it always reports port 2. Need to check it as well

@keboliu keboliu merged commit b0b4a41 into keboliu:sonic_alligator_bu_master Jan 26, 2022
@stephenxs stephenxs deleted the sonic_alligator_bu_master_rj45 branch January 26, 2022 03:45
keboliu pushed a commit that referenced this pull request Mar 21, 2022
f00efef Longxiang Lyu Wed Mar 16 09:12:46 2022 +0800 Add a command line option to store logs into a separate file (#41)
ff2e67d Longxiang Lyu Tue Mar 15 09:10:59 2022 +0800 Add default port cable type (#39)
ebbb4d8 Jing Zhang Mon Mar 14 15:41:11 2022 -0700 Prevent switching MUX to "Unknown" (#36)
c779b8f Longxiang Lyu Thu Mar 10 21:35:11 2022 +0800 [nonfunctional] Use LinkProberStateMachineBase (#38)
b9fedd0 Longxiang Lyu Wed Mar 9 13:03:58 2022 +0800 [NONFUNCTIONAL] Add LinkProberStateMachineBase (#37)
bedd42b Longxiang Lyu Wed Mar 9 10:03:00 2022 +0800 Add .clang-format file to format code (#28)
9fe4fc6 Guohan Lu Thu Mar 3 17:51:43 2022 -0800 [doc]: add lgtm badge in README.md
c1249d9 Longxiang Lyu Wed Mar 2 18:05:18 2022 +0800 Enable lgtm (#33)
b8514c6 Longxiang Lyu Wed Mar 2 13:34:39 2022 +0800 Collect port cable type to use corresponding state machine (#31)
9b59ef9 Longxiang Lyu Wed Mar 2 07:19:33 2022 +0800 Improve make clean (#32)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants