Transceiver_101_103_fix#5641
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces support for Cisco 800G ZR and ZR+ optics within the platform testing framework. It includes necessary updates to interface configuration logic, metadata definitions, and telemetry validation routines to ensure compatibility with Cisco-specific hardware behaviors and operational modes. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces vendor-specific handling for Cisco 800G ZR/ZR+ optics, including custom operational modes, fallback transceiver form-factor decoding, and support for new platform exceptions. It also refines telemetry validation by skipping detailed component checks when interfaces are in a DOWN state. Feedback on the changes includes a recommendation to use gnmi.Lookup instead of gnmi.Get to prevent test failures when retrieving the transceiver form factor, and a suggestion to remove a redundant index assignment block in the interface configuration plugin.
| if deviations.EthChannelAssignmentCiscoNumbering(dut) { | ||
| assignment[0].Index = ygot.Uint32(1) | ||
| assignment[assignmentIndex].Index = ygot.Uint32(1) | ||
| } |
There was a problem hiding this comment.
The if deviations.EthChannelAssignmentCiscoNumbering(dut) block is redundant. When EthChannelAssignmentCiscoNumbering is true, assignmentIndex is set to 1, and the map entry is initialized with Index: ygot.Uint32(assignmentIndex) (which is 1). Therefore, explicitly setting assignment[assignmentIndex].Index = ygot.Uint32(1) again is unnecessary. This block can be safely removed.
| directFormFactor := gnmi.Get(t, dut, gnmi.OC().Component(params.TransceiverNames[p.Name()]).Transceiver().FormFactor().State()) | ||
| if directFormFactor != oc.TransportTypes_TRANSCEIVER_FORM_FACTOR_TYPE_UNSET { | ||
| formFactor = directFormFactor | ||
| } |
There was a problem hiding this comment.
Using gnmi.Get directly can cause the test to fail immediately if the form-factor leaf is not present or supported on the device. Using gnmi.Lookup is more robust as it allows checking if the value is present before using it.
| directFormFactor := gnmi.Get(t, dut, gnmi.OC().Component(params.TransceiverNames[p.Name()]).Transceiver().FormFactor().State()) | |
| if directFormFactor != oc.TransportTypes_TRANSCEIVER_FORM_FACTOR_TYPE_UNSET { | |
| formFactor = directFormFactor | |
| } | |
| if directFormFactorVal, present := gnmi.Lookup(t, dut, gnmi.OC().Component(params.TransceiverNames[p.Name()]).Transceiver().FormFactor().State()).Val(); present { | |
| if directFormFactorVal != oc.TransportTypes_TRANSCEIVER_FORM_FACTOR_TYPE_UNSET { | |
| formFactor = directFormFactorVal | |
| } | |
| } |
2 participants
No description provided.