Skip to content

Add transceiver status to wicket#7562

Merged
bnaecker merged 6 commits intomainfrom
show-transceiver-info-in-wicket
Feb 22, 2025
Merged

Add transceiver status to wicket#7562
bnaecker merged 6 commits intomainfrom
show-transceiver-info-in-wicket

Conversation

@bnaecker
Copy link
Copy Markdown
Collaborator

- Make the current inventory specific to MGS
- Add per-switch transceiver list to the inventory as well
- Fetch transceiver state periodically in wicketd
- Draw transceiver details in the wicket inventory panel
- Fixes #6710
@bnaecker
Copy link
Copy Markdown
Collaborator Author

Here is a screenshot showing the normal, expected contents when a transceiver is all working correctly.

Screenshot 2025-02-19 at 14 13 34

There are a few ways the transceivers can go off the rails, but the most common is that we simply have no light on one side or the other. Below is a screenshot of what that will look like if we have transmit power below some minimum threshold. (I simulated this by disabling the link via Dendrite.) We'll have similarly-colored warnings for all the monitors, if they're outside some expected range.

Screenshot 2025-02-19 at 14 13 53

This last part isn't perfect. One could argue that we shouldn't consider it unusual if there's no Tx power when the link is disabled entirely -- we shouldn't be transmitting anything. There are also some other flags and warnings we can print, such as whether we have a Tx fault, or if the module can recover a clock from the Rx signal. I'm open to adding these in later PRs if we think they'll be useful.

@bnaecker bnaecker requested review from jgallagher and sunshowers and removed request for jgallagher February 19, 2025 22:16
Copy link
Copy Markdown
Contributor

@sunshowers sunshowers left a comment

Choose a reason for hiding this comment

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

Thank you! Just a few questions.

- Make transceiver data optional, so we report what we can. Handle
  optional values when printing in `wicket` UI.
- Add comments about how we're addressing transceiver modules when
  collecting all the data and how that fails.
- Add comment around transceiver interface naming.
- Add comment and clarify `wicket` UI text around software override of
  the module hardware power signal
Rename `inventory_or_unavail` to `mgs_inventory_or_unavail`
- Update OpenAPI for new optional transceiver bits
- Simplify nested options with a result, making it clearer when we have
  failed to read transceiver information and when we read it, but the
  module doesn't support it.
@bnaecker
Copy link
Copy Markdown
Collaborator Author

I'd like to test this one more time on a system with actual transceivers, since 1d87e93 updated the logic around UI formatting quite a bit. Will take this for another lap when a system opens up in the lab.

@bnaecker
Copy link
Copy Markdown
Collaborator Author

Ok, running the latest on dublin shows that things still look reasonable from a UI perspective. I've updated a few pieces of specific wording, like "software control" instead of "software override" or "unsupported" instead of "unavailable". But otherwise, this is the same.

Screenshot 2025-02-21 at 15 31 21

@bnaecker
Copy link
Copy Markdown
Collaborator Author

So funny enough, the temp and voltage being unsupported uncovered a bug, thanks to @rmustacc. See oxidecomputer/transceiver-control#339, which is fixed in oxidecomputer/transceiver-control#341. That will get picked up by this code automatically next time we update the transceiver-control crate, no need to block this on that right now.

Copy link
Copy Markdown
Contributor

@sunshowers sunshowers left a comment

Choose a reason for hiding this comment

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

Awesome, thank you so much for working through the comments!

@bnaecker bnaecker enabled auto-merge (squash) February 22, 2025 01:51
@bnaecker bnaecker merged commit f11b2c3 into main Feb 22, 2025
18 checks passed
@bnaecker bnaecker deleted the show-transceiver-info-in-wicket branch February 22, 2025 05:28
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.

Wicket could show basic transceiver status during rack setup

2 participants