[RFC] Add notification support for relay services

[kurtjd]

There was some discussion offline on if notifications should be handled as broadcast messages, but I think we can keep it pretty simple if we just keep notifications limited to point-to-point (as in, service -> relay).

This allows us to not need to know statically how many publishers there are (which, since a PubSubChannel is type stated over the number of publishers, this knowledge would need to bleed into the interface and all the services that need to publish notifications) The way around that would be to use dynamic senders/receivers from pubsub (which erases that particular type state) but I think it's best if we avoid dynamic dispatch if we can help it.

Thus, I decided on a regular Channel, with the intention that the relay service is the sole listener and we can create as many Notifiers as we want to at run time. My idea is if services also need to notify other services when they signal a notification, that can be handled by each particular service as it sees fit (similar to how power-policy broadcasts messages).

Mentioned in the doc strings, I use a Channel with a fixed size of 1 so that typestate is further hidden from the interface, and I think that is justified because notifications are pretty rare events, thus multiple services raising notifications simultaneously should be even rarer. The channel at least gives us backpressure so that no notification is lost if it does happen, and as mentioned the performance penalty should be negligible if backpressure is encountered.

Another reason I wanted to keep notifications specific to only the relay service (and not intended as broadcast messages) is because notifications are not intended to hold payloads, and the notification id would be meaningless to other services. The id is quite platform specific and only the relay service would know what to do with it. For example, the current espi-service would interpret it as an IRQ offset, and the future hid-service would interpret it as a specific GPIO line to make active. Thus, when a new Notifier is created, basically the caller has to know how the provided id will be used by whichever relay service they are choosing to use on their platform, and how that will ultimately bubble up to the host.

Finally, I decided to add a method to the RelayHandler trait for getting a reference to the notification listener, mainly just to make it easier for instantiating the relay service (so we only have to pass in the relay handler, and not the notification listener additionally).

This PR is currently RFC to get input, and I've quickly integrated it into espi-service and thermal-service to demonstrate how it would be used. When ready for merge, I will remove the espi and thermal service integrations and make those more fleshed out PRs. Fully integrated on a platform it would look like:

embedded_services::relay::mctp::impl_odp_mctp_relay_handler!(
    OdpRelayHandler;
    Battery, 0x08, battery_service::Service;
    Thermal, 0x09, thermal_service::Service<'static>;
    TimeAlarm, 0x0B, time_alarm_service::Service<'static>;
);

embedded_services::init().await;

// Setup notification handler
static NH: StaticCell<embedded_services::relay::notifications::NotificationHandler> = StaticCell::new();
let notification_handler = embedded_services::relay::notifications::NotificationHandler::new();
let notification_handler = NH.init(notification_handler);

// Configure sensors, fans, etc...
// Here we have to know that espi is our relay handler, and that an ID of 0x1 would correspond to an IRQ offset for espi,
// and that the host SoC understands that this means a thermal notification.
let thermal = ts::Service::init(&STORAGE, sensors, fans, notification_handler.new_notifier(0x1)).await;

// Configure and setup battery service...
// Configure and setup time-alarm service...

// We create the sole Listener and pass it to the relay handler
let relay_handler = OdpRelayHandler::new(notification_handler.new_listener().unwrap(), battery, thermal, time_alarm);

// Configure espi peripheral...
static ESPI_SERVICE: OnceLock<espi_service::Service<OdpRelayHandler<'static>>> = OnceLock::new();
let espi_service = espi_service::Service::init(&ESPI_SERVICE, espi, relay_handler).await;

[Copilot]

Pull request overview

Adds a notification mechanism for relay services so non-payload “attention” events can flow from individual services to the active relay (e.g., eSPI), and demonstrates usage in thermal + eSPI.

Changes:

• Extends RelayHandler with a notification_handler() accessor and updates the ODP relay-handler macro to carry a notification handler.
• Introduces relay::notifications (NotificationHandler + Notifier) built on an embassy_sync::Channel<u8, 1>.
• Integrates notifications into espi-service (IRQ push on notification) and thermal-service (notify host on threshold events).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File	Description
thermal-service/src/lib.rs	Adds a Notifier dependency to the thermal service and emits host notifications on threshold events.
espi-service/src/espi_service.rs	Listens for notifications alongside eSPI events/host TX queue and pushes IRQs to the host.
embedded-service/src/relay/mod.rs	Adds the notification API/module and wires it into the relay handler trait + generator macro.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[jerrysxie]

@kurtjd I am a little confused over the direction of this notification. How is this different from a service trying to send the host a message through the relay service? I thought this notification is going this direction: host -> relay service -> corresponding EC service.

If it is intending for EC service -> relay service -> host, can we just use the existing mechanism to send a message?

[kurtjd]

@kurtjd I am a little confused over the direction of this notification. How is this different from a service trying to send the host a message through the relay service? I thought this notification is going this direction: host -> relay service -> corresponding EC service.

If it is intending for EC service -> relay service -> host, can we just use the existing mechanism to send a message?

Notifications are meant to follow this path:
Service -> Relay service -> Host

For example, the way it would work with debug service is:

• Debug serivce has a defmt frame available
• Debug service notifies relay service
• Relay service converts this notification into something like an eSPI IRQ offset
• The host sees this eSPI interrupt, and sends down a request to read the frame
• The relay service sends this request to the debug service via direct async call
• The debug service responds with the defmt frame

Without comms, there is currently no way for a service to talk to the relay service. Currently the relay service will make direct async calls on a service when the host sends a request, and the other services can only respond to this direct async call.

This is meant to make it so services can initiate communications with the relay service and not just respond to a request. Without comms, or message system like ergot, we don't have a generic way for services to pass messages along to other services.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[jerrysxie]

@kurtjd I am a little confused over the direction of this notification. How is this different from a service trying to send the host a message through the relay service? I thought this notification is going this direction: host -> relay service -> corresponding EC service.
If it is intending for EC service -> relay service -> host, can we just use the existing mechanism to send a message?

Notifications are meant to follow this path: Service -> Relay service -> Host

For example, the way it would work with debug service is:

• Debug serivce has a defmt frame available
• Debug service notifies relay service
• Relay service converts this notification into something like an eSPI IRQ offset
• The host sees this eSPI interrupt, and sends down a request to read the frame
• The relay service sends this request to the debug service via direct async call
• The debug service responds with the defmt frame

Without comms, there is currently no way for a service to talk to the relay service. Currently the relay service will make direct async calls on a service when the host sends a request, and the other services can only respond to this direct async call.

This is meant to make it so services can initiate communications with the relay service and not just respond to a request. Without comms, or message system like ergot, we don't have a generic way for services to pass messages along to other services.

Gotcha, I have my directions swapped in my mental model when reviewing this. Our EC communication are host centric.

[kurtjd]

@kurtjd I am a little confused over the direction of this notification. How is this different from a service trying to send the host a message through the relay service? I thought this notification is going this direction: host -> relay service -> corresponding EC service.
If it is intending for EC service -> relay service -> host, can we just use the existing mechanism to send a message?

Notifications are meant to follow this path: Service -> Relay service -> Host
For example, the way it would work with debug service is:

• Debug serivce has a defmt frame available
• Debug service notifies relay service
• Relay service converts this notification into something like an eSPI IRQ offset
• The host sees this eSPI interrupt, and sends down a request to read the frame
• The relay service sends this request to the debug service via direct async call
• The debug service responds with the defmt frame

Without comms, there is currently no way for a service to talk to the relay service. Currently the relay service will make direct async calls on a service when the host sends a request, and the other services can only respond to this direct async call.
This is meant to make it so services can initiate communications with the relay service and not just respond to a request. Without comms, or message system like ergot, we don't have a generic way for services to pass messages along to other services.

Gotcha, I have my directions swapped in my mental model when reviewing this. Our EC communication are host centric.

It still does raise a good question how best to handle two services that need to talk to eachother now that we are moving away from comms. As Robert pointed out in his presentation, we can't have service A make direct calls to service B and service B make direct calls to service A due to the circular dependency. I feel this might be something we run into with other services that BOTH need to make requests to eachother and maybe we can standardize on something here.

[jerrysxie]

@kurtjd I am a little confused over the direction of this notification. How is this different from a service trying to send the host a message through the relay service? I thought this notification is going this direction: host -> relay service -> corresponding EC service.
If it is intending for EC service -> relay service -> host, can we just use the existing mechanism to send a message?

Notifications are meant to follow this path: Service -> Relay service -> Host
For example, the way it would work with debug service is:

• Debug serivce has a defmt frame available
• Debug service notifies relay service
• Relay service converts this notification into something like an eSPI IRQ offset
• The host sees this eSPI interrupt, and sends down a request to read the frame
• The relay service sends this request to the debug service via direct async call
• The debug service responds with the defmt frame

Without comms, there is currently no way for a service to talk to the relay service. Currently the relay service will make direct async calls on a service when the host sends a request, and the other services can only respond to this direct async call.
This is meant to make it so services can initiate communications with the relay service and not just respond to a request. Without comms, or message system like ergot, we don't have a generic way for services to pass messages along to other services.

Gotcha, I have my directions swapped in my mental model when reviewing this. Our EC communication are host centric.

It still does raise a good question how best to handle two services that need to talk to eachother now that we are moving away from comms. As Robert pointed out in his presentation, we can't have service A make direct calls to service B and service B make direct calls to service A due to the circular dependency. I feel this might be something we run into with other services that BOTH need to make requests to eachother and maybe we can standardize on something here.

I believe for service that needs to the talk with each other, with the current migration away from comms, the planned approach is to define some traits, and have the services register their sender and receiver with each during instantiation.

@RobertZ2011 @williampMSFT Please correct me if I am wrong.

[kurtjd]

@kurtjd I am a little confused over the direction of this notification. How is this different from a service trying to send the host a message through the relay service? I thought this notification is going this direction: host -> relay service -> corresponding EC service.
If it is intending for EC service -> relay service -> host, can we just use the existing mechanism to send a message?

Notifications are meant to follow this path: Service -> Relay service -> Host
For example, the way it would work with debug service is:

• Debug serivce has a defmt frame available
• Debug service notifies relay service
• Relay service converts this notification into something like an eSPI IRQ offset
• The host sees this eSPI interrupt, and sends down a request to read the frame
• The relay service sends this request to the debug service via direct async call
• The debug service responds with the defmt frame

Without comms, there is currently no way for a service to talk to the relay service. Currently the relay service will make direct async calls on a service when the host sends a request, and the other services can only respond to this direct async call.
This is meant to make it so services can initiate communications with the relay service and not just respond to a request. Without comms, or message system like ergot, we don't have a generic way for services to pass messages along to other services.

Gotcha, I have my directions swapped in my mental model when reviewing this. Our EC communication are host centric.

It still does raise a good question how best to handle two services that need to talk to eachother now that we are moving away from comms. As Robert pointed out in his presentation, we can't have service A make direct calls to service B and service B make direct calls to service A due to the circular dependency. I feel this might be something we run into with other services that BOTH need to make requests to eachother and maybe we can standardize on something here.

I believe for service that needs to the talk with each other, with the current migration away from comms, the planned approach is to define some traits, and have the services register their sender and receiver with each during instantiation.

@RobertZ2011 @williampMSFT Please correct me if I am wrong.

Got it. Though ultimately it then makes me wonder why not just standardize on something like ergot? It seems direct async calls are only applicable in a few circumstances, and they can continue to be used where they make sense, but it still seems like we need some standard way to pass messages back and forth between services.

[RobertZ2011]

We should avoid circular dependencies among services. Even with a message passing approach, you run into problems with reentrancy. If service A calls into service B then B can't call into A because A is already blocked waiting on B. This applies both to using function calls and to message passing.

The first thing would be to consider if the two services actually need to be separate. If they're this interdependent then the right design might be just to combine them. If they need to remain separate then I would see about moving anything that needs to be shared into its own struct. You could also invert this approach and have the shared struct use A and B, instead of A and B using the shared struct.

[kurtjd]

We should avoid circular dependencies among services. Even with a message passing approach, you run into problems with reentrancy. If service A calls into service B then B can't call into A because A is already blocked waiting on B. This applies both to using function calls and to message passing.

The first thing would be to consider if the two services actually need to be separate. If they're this interdependent then the right design might be just to combine them. If they need to remain separate then I would see about moving anything that needs to be shared into its own struct. You could also invert this approach and have the shared struct use A and B, instead of A and B using the shared struct.

Got it. I think then my approach for notifications matches what you are thinking. Relay services and other services must be circularly dependent because relay services need to talk to other services when the host sends down a request and other services need to talk to the relay service when they need to notify the host.

So my approach of using a NotificationHandler struct as the intermediate between relay services and other services seems to match what you are thinking then, if I'm understanding correctly?

[williampMSFT]

This looks odd to me - it seems like this would prevent other services from also receiving these notifications, right? Why do we have this limitation?

[kurtjd]

Yeah I talked about this briefly in the PR description/doc strings. I decided to limit this to only relay services because there is no payload and the id is only meaningful to the relay services. So since we only have one relay service at a time, we limit this to only one listener. My idea was if services also need to notify other services, that would be through a separate stream since it is difficult to handle all that here (such as needing to expose the number of listeners/publishers in the interface like you pointed out) and because they would likely want to send a payload along with the notification to the other service.

[kurtjd]

As in, if TAS needs to notify the host AND notify another service, it would use this interface to notify the host then rely on some mechanism established explicitly between the TAS and that other service.

If we are trying to make this generic enough to send any arbitrary payload to any other service it feels like we are just reimplementing a message passing system, so I'm not sure that's the route we are trying to go.

[williampMSFT]

Chatted about this offline, but for the benefit of other reviewers - I think the plan is to look into doing something where notifications go over something like a PubSubChannel and then extend the RelayServiceHandlerTypes / RelayServiceHandler traits to describe how to translate notifications on that PubSubChannel into something that goes over the relay bus. This should allow individual services to use the same notifications for host communication and service-to-service communication without caring where they're going, and it isolates knowledge of what the host does and doesn't care about in the same place as all the serialization/deserialization logic