-
Notifications
You must be signed in to change notification settings - Fork 0
Robot Agnostic Foxglove #108
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds robot-agnostic support to the System Status Panel and Discrete Servos Panel, enabling these panels to adapt their display based on the selected robot (Crush or Oogway). It also deprecates the Call Service Panel and Subscribe Topic Panel by removing them entirely from the codebase.
Key changes:
- Refactored System Status Panel to support robot-specific configurations with topic timeout detection and click-to-copy functionality
- Updated Discrete Servos Panel to display robot-specific servo controls
- Removed Call Service Panel and Subscribe Topic Panel extensions
- Updated documentation to reflect deprecated panels
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| foxglove/extensions/system-status-panel/src/SystemStatusPanel.tsx | Refactored to support robot-agnostic configuration, added topic timeout detection, click-to-copy functionality, and robot name validation |
| foxglove/extensions/discrete-servos-panel/src/DiscreteServosPanel.tsx | Added robot-agnostic support with robot-specific servo configurations |
| foxglove/extensions/sensors-status-panel/src/SensorsStatusPanel.tsx | Reordered Tooltip and TableRow components for consistency |
| foxglove/extensions/subscribe-topic-panel/* | Removed all files to deprecate the panel |
| foxglove/extensions/call-service-panel/* | Removed all files to deprecate the panel |
| foxglove/extensions/publish-topic-panel/README.md | Enhanced documentation with feature list and use cases |
| foxglove/README.md | Updated extensions list to remove deprecated panels |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
foxglove/extensions/system-status-panel/src/SystemStatusPanel.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
foxglove/extensions/system-status-panel/src/SystemStatusPanel.tsx
Outdated
Show resolved
Hide resolved
foxglove/extensions/sensors-status-panel/src/SensorsStatusPanel.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
foxglove/extensions/discrete-servos-panel/src/DiscreteServosPanel.tsx:27
- The type name
DiscreteServosPanelconflicts with the function nameDiscreteServosPanel. Consider renaming the type toDiscreteServosPanelStateto match the naming pattern used elsewhere and avoid confusion.
type DiscreteServosPanel = {
message?: string; // Response from service call
success?: boolean; // Whether the service call was successful
};
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@maxwellmlin What is the alternative to the call service panel? Is the goal to have dedicated panels for all services we might want to call (i.e., servos)? |
@nathanaelren Foxglove already has the Service Call and Raw Messages panel which does the same thing as our Call Service Panel and Subscribe Topic Panel. This confuses new members so I think we should remove them. (Although Foxglove has a Publish panel, our Publish Topic Panel is better so we're keeping that one.) |
@maxwellmlin Got it, thanks. Agree on our publish topic panel. I think the |
@nathanaelren I think it's OK to omit. The Foxglove Service Call panel pre-populates the request JSON so I don't think we need an example anymore. |
nathanaelren
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
Sounds good, thanks. Seems reasonable to me, we can merge in |
Add automatic deployment of Foxglove extensions upon pushes to
main.productionenvironment withFOXGLOVE_API_KEY.Add robot agnostic support to
System Status Panel
Call Service Panel
Subscribe Topic Panel
Add buttons to automatically set ROBOT_NAME if it is not set.