-
Notifications
You must be signed in to change notification settings - Fork 44
Refactor MCTP relay boilerplate into macro, clarify result nomenclature #673
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
Refactor MCTP relay boilerplate into macro, clarify result nomenclature #673
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 refactors the MCTP relay boilerplate into a reusable macro and standardizes nomenclature to distinguish "Result" (the generic Result enum type) from "Response" (the success case of a Result). The macro consolidates repetitive code for generating relay types across different message services.
Changes:
- Introduced
impl_odp_mctp_relay_types!macro to generate ODP MCTP relay boilerplate from service specifications - Renamed
SerializableResponsetoSerializableResultthroughout the codebase - Renamed
HostResponsetoHostResultand related types/functions - Added type aliases (
AcpiBatteryResult,ThermalResult,DebugResult) for service result types
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
| embedded-service/src/relay/mod.rs | Added impl_odp_mctp_relay_types! macro to generate MCTP relay types; renamed SerializableResponse to SerializableResult |
| espi-service/src/mctp.rs | Replaced manual type definitions with macro invocation for Battery, Thermal, and Debug services |
| espi-service/src/espi_service.rs | Updated type names from HostResponse to HostResult and HostResponseMessage to HostResultMessage; renamed function try_route_request_to_comms to send_to_comms |
| battery-service-messages/src/lib.rs | Added AcpiBatteryResult type alias for Result<AcpiBatteryResponse, AcpiBatteryError> |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
This change factors out the boilerplate associated with generating relay types from the information about the set of types that are to be relayed. This should enable easier bringup of additional message relay services (e.g. UART).
In the course of this work, I noticed that some of the nomenclature around results vs responses was a bit conflicting in the relay code, so also standardized that on Result meaning 'a specific generic type of the Result enum' and Response meaning 'the success case of a Result'.