Add .github/copilot-instructions.md by jerrysxie · Pull Request #744 · OpenDevicePartnership/embedded-services

jerrysxie · 2026-03-09T16:51:58Z

Add Copilot instructions covering build/test/lint commands, service architecture patterns, and key codebase conventions.

Add Copilot instructions covering build/test/lint commands, service architecture patterns, and key codebase conventions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Adds a new .github/copilot-instructions.md document intended to guide Copilot (and contributors) on how to build/test/lint the workspace and on key architecture/convention details for the embedded-services codebase.

Changes:

Introduces a Copilot instruction doc with common build/lint/test commands aligned to CI workflows.
Documents service/IPC architecture concepts (comms endpoints, message routing) and core utilities/macros.
Captures conventions around feature flags, clippy strictness, dependencies, and testing.

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

Copilot · 2026-03-09T16:55:51Z

.github/copilot-instructions.md

+Each service crate follows a consistent structure:
+
+1. **Service struct** with a `comms::Endpoint` and domain-specific context/state
+2. **`MailboxDelegate` impl** — the `receive()` method handles incoming messages using type-safe downcasting via `message.data.get::<T>()`
+3. **Global singleton** — services are stored in `OnceLock<Service>` statics
+4. **Async task function** — registers the endpoint, then loops calling a `process()` or `process_next()` method
+5. **Spawned via Embassy** — `#[embassy_executor::task]` functions are spawned from main


The “Service Pattern” section describes a single comms/Endpoint + MailboxDelegate-based structure as if it applies to every service crate. In this repo, some services (e.g., type-c-service) use other IPC patterns (pubsub/broadcaster, ipc::deferred) and don’t have a comms::Endpoint field. Consider rewording this section to describe the comms-based pattern as common (not universal) and call out the alternative patterns used elsewhere in the workspace.

Copilot · 2026-03-09T16:55:51Z

.github/copilot-instructions.md

+
+pub async fn task() {
+    let service = SERVICE.get_or_init(MyService::new);
+    comms::register_endpoint(service, &service.endpoint).await.unwrap();


The service skeleton example uses .await.unwrap() when registering the endpoint, but later this doc states clippy denies unwrap_used/no panicking in production code. To avoid sending mixed signals, update the example to handle the Result explicitly (e.g., propagate or log/return) rather than unwrapping.

Suggested change

comms::register_endpoint(service, &service.endpoint).await.unwrap();

if let Err(err) = comms::register_endpoint(service, &service.endpoint).await {

// TODO: handle registration error (e.g., log and return or trigger a safe reset)

return;

}

Copilot · 2026-03-09T16:55:52Z

.github/copilot-instructions.md

+
+```shell
+# ARM board examples
+cd examples/rt685s-evk && cargo clippy --target thumbv8m.main-none-eabihf --locked


The examples build/lint section omits the examples/rt633 workspace, which is present in the repo and is also checked in CI. Add an example command for examples/rt633 (or generalize the instructions to cover all examples/* workspaces).

Suggested change

cd examples/rt685s-evk && cargo clippy --target thumbv8m.main-none-eabihf --locked

cd examples/rt685s-evk && cargo clippy --target thumbv8m.main-none-eabihf --locked

cd examples/rt633 && cargo clippy --target thumbv8m.main-none-eabihf --locked

Copilot · 2026-03-09T16:55:52Z

.github/copilot-instructions.md

+- Custom `enum` error types per module — no `thiserror` (it requires std)
+- All error enums derive `Debug, Clone, Copy, PartialEq, Eq`


The error-handling conventions are stated too strongly: not all error types are enums and not all derive Clone, Copy, PartialEq, Eq (e.g., some error enums only derive Debug/Copy, and some services use a struct error). Consider changing this to “prefer” these derives when practical, and note that some modules use lightweight struct errors.

Suggested change

- Custom `enum` error types per module — no `thiserror` (it requires std)

- All error enums derive `Debug, Clone, Copy, PartialEq, Eq`

- Prefer custom `enum` error types per module — no `thiserror` (it requires std); some modules instead use lightweight struct error types when that is a better fit

- Prefer deriving `Debug, Clone, Copy, PartialEq, Eq` on error enums when practical (some errors may only derive a subset, e.g., `Debug`/`Copy`)

Copilot · 2026-03-09T16:55:53Z

.github/copilot-instructions.md

+- Async tests use `embassy_futures::block_on(async { ... })`
+- Dev-dependencies enable `std` features: `embassy-sync/std`, `embassy-time/std`, `critical-section/std`
+- `tokio` with `rt`, `macros`, `time` features for integration tests


Testing guidance says async tests use embassy_futures::block_on(async { ... }), but there are many async unit tests written with #[tokio::test] in this repo. It would be more accurate to mention both patterns and when each is used.

Suggested change

- Async tests use `embassy_futures::block_on(async { ... })`

- Dev-dependencies enable `std` features: `embassy-sync/std`, `embassy-time/std`, `critical-section/std`

- `tokio` with `rt`, `macros`, `time` features for integration tests

- Async unit tests in `no_std`/Embassy-focused crates use `embassy_futures::block_on(async { ... })` to stay runtime-agnostic

- Host-only async tests and integration tests may use `#[tokio::test]` in crates that depend on `tokio`

- Dev-dependencies enable `std` features: `embassy-sync/std`, `embassy-time/std`, `critical-section/std`

- `tokio` with `rt`, `macros`, `time` features is used to support `#[tokio::test]`-based host/integration tests

Copilot · 2026-03-09T16:55:53Z

.github/copilot-instructions.md

+
+### Core Utilities (embedded-service crate)
+
+- **`GlobalRawMutex`**: `ThreadModeRawMutex` on ARM bare-metal, `CriticalSectionRawMutex` in tests/std


GlobalRawMutex is described as ThreadModeRawMutex on ARM bare-metal and CriticalSectionRawMutex in tests/std, but the actual cfg also selects CriticalSectionRawMutex on riscv32. Consider mentioning that exception to avoid confusing cross-target contributors.

Suggested change

- **`GlobalRawMutex`**: `ThreadModeRawMutex` on ARM bare-metal, `CriticalSectionRawMutex` in tests/std

- **`GlobalRawMutex`**: `ThreadModeRawMutex` on ARM bare-metal, `CriticalSectionRawMutex` on RISC-V bare-metal and in tests/std

.github/copilot-instructions.md

kurtjd

I'll be honest I'm still not entirely sure what these instructions offer if they are AI generated in the first place (I thought things like skills, AGENTS.md, CLAUDE.md, etc are to provide additional context the LLM can't glean on it's own from the codebase) but that likely means I just need to research this a bit more.

jerrysxie · 2026-03-11T15:03:23Z

I'll be honest I'm still not entirely sure what these instructions offer if they are AI generated in the first place (I thought things like skills, AGENTS.md, CLAUDE.md, etc are to provide additional context the LLM can't glean on it's own from the codebase) but that likely means I just need to research this a bit more.

@kurtjd It is more like breadcrumbs to help the agent to find its way. Unless we commit something to AI agent's memory, it will need to rediscover the context every single time when we open a new session. Instruction file helps AI to find its way faster. We can also add contributing policy to the instruction file so agent would follow our guidelines.

kurtjd · 2026-03-11T15:16:11Z

I'll be honest I'm still not entirely sure what these instructions offer if they are AI generated in the first place (I thought things like skills, AGENTS.md, CLAUDE.md, etc are to provide additional context the LLM can't glean on it's own from the codebase) but that likely means I just need to research this a bit more.

@kurtjd It is more like breadcrumbs to help the agent to find its way. Unless we commit something to AI agent's memory, it will need to rediscover the context every single time when we open a new session. Instruction file helps AI to find its way faster. We can also add contributing policy to the instruction file so agent would follow our guidelines.

Okay that makes sense to me. Yeah I suppose we just need to be careful since embedded-services patterns and architecture are changing pretty frequently and don't want this to drift too far from the actual state of the repo. Maybe a rolling workflow can have Copilot regenerate this every so often or just have a rule that PRs introducing significant changes need to update this as well.

williampMSFT · 2026-03-11T17:12:54Z

.github/copilot-instructions.md

+
+Each service crate follows a consistent structure:
+
+1. **Service struct** with a `comms::Endpoint` and domain-specific context/state


I think we're in the process of making 1-3 here not true; we've already migrated a bunch of services away from the comms system and out of OnceLock statics. May want to just remove those pieces

williampMSFT · 2026-03-11T17:14:06Z

.github/copilot-instructions.md

+
+### Communication (IPC)
+
+Services communicate through `embedded_services::comms` — a type-erased message routing system built on intrusive linked lists (zero allocation):


same here - comms system is going away

Add .github/copilot-instructions.md

46d05f9

Add Copilot instructions covering build/test/lint commands, service architecture patterns, and key codebase conventions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

jerrysxie self-assigned this Mar 9, 2026

jerrysxie added the enhancement New feature or request label Mar 9, 2026

Copilot AI review requested due to automatic review settings March 9, 2026 16:51

jerrysxie requested a review from a team as a code owner March 9, 2026 16:51

jerrysxie added this to ODP Backlog Mar 9, 2026

jerrysxie requested review from RobertZ2011, felipebalbi, tullom and williampMSFT March 9, 2026 16:52

Copilot started reviewing on behalf of jerrysxie March 9, 2026 16:52 View session

Copilot AI reviewed Mar 9, 2026

View reviewed changes

kurtjd requested changes Mar 9, 2026

View reviewed changes

.github/copilot-instructions.md Show resolved Hide resolved

.github/copilot-instructions.md Show resolved Hide resolved

kurtjd approved these changes Mar 11, 2026

View reviewed changes

williampMSFT reviewed Mar 11, 2026

View reviewed changes

jerrysxie moved this to In progress in ODP Backlog Mar 12, 2026

-    comms::register_endpoint(service, &service.endpoint).await.unwrap();
+    if let Err(err) = comms::register_endpoint(service, &service.endpoint).await {
+        // TODO: handle registration error (e.g., log and return or trigger a safe reset)
+        return;
+    }

	cd examples/rt685s-evk && cargo clippy --target thumbv8m.main-none-eabihf --locked
	cd examples/rt685s-evk && cargo clippy --target thumbv8m.main-none-eabihf --locked
	cd examples/rt633 && cargo clippy --target thumbv8m.main-none-eabihf --locked

		- Custom `enum` error types per module — no `thiserror` (it requires std)
		- All error enums derive `Debug, Clone, Copy, PartialEq, Eq`


		### Core Utilities (embedded-service crate)

		- `GlobalRawMutex`: `ThreadModeRawMutex` on ARM bare-metal, `CriticalSectionRawMutex` in tests/std


		Each service crate follows a consistent structure:

		1. Service struct with a `comms::Endpoint` and domain-specific context/state


		### Communication (IPC)

		Services communicate through `embedded_services::comms` — a type-erased message routing system built on intrusive linked lists (zero allocation):

Conversation

jerrysxie commented Mar 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kurtjd left a comment

Choose a reason for hiding this comment

Uh oh!

jerrysxie commented Mar 11, 2026

Uh oh!

kurtjd commented Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

williampMSFT Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

williampMSFT Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kurtjd commented Mar 11, 2026 •

edited

Loading