Use NotificationBits in Idol#2233
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR updates the NotificationHandler trait implementations across the codebase to use the new NotificationBits type instead of raw u32 values. The change provides type safety and better APIs for notification handling.
- Replaces raw
u32notification bits withuserlib::NotificationBitstype - Updates notification bit checking to use new type-safe methods like
is_timer_set()andis_set_unconditional() - Removes unused timer functionality from
medusa-seq-server
Reviewed Changes
Copilot reviewed 54 out of 55 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| Multiple task/drv files | Update handle_notification method signatures to use NotificationBits |
| sys/userlib/src/lib.rs | Add new methods to NotificationBits type for convenient bit checking |
| drv/medusa-seq-server/src/main.rs | Remove timer-related code that wasn't being used |
| task/host-sp-comms/src/main.rs | Mix of new API usage and raw bits access for timer handling |
| Cargo.toml | Update idolatry dependencies to use feature branch |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
879286c to
e572a5a
Compare
0e2977a to
db707eb
Compare
e572a5a to
846641d
Compare
db707eb to
5f2cd34
Compare
846641d to
70d84fe
Compare
4dae22b to
d618c40
Compare
70d84fe to
1ab0600
Compare
d618c40 to
737af5f
Compare
1ab0600 to
1649070
Compare
737af5f to
fb69232
Compare
1649070 to
9e5e7ef
Compare
3f913aa to
e86c0c1
Compare
3cfc21b to
eaedd40
Compare
e86c0c1 to
b0cc370
Compare
b91ad4a to
c2dcd77
Compare
| } | ||
|
|
||
| if (bits & notifications::TIMER_MASK) != 0 { | ||
| if bits.has_timer_fired(notifications::TIMER_MASK) { |
There was a problem hiding this comment.
This is technically a functionality change, since it looks like I missed this code when adding the timer checks. But I think the result is correct.
There was a problem hiding this comment.
IIRC you intentionally skipped it because I was also messing with stuff in #2228 😄
c2dcd77 to
bc60ff2
Compare
This is mostly mechanical, with one logical change: I removed the timer from
medusa-seq-server, because it wasn't doing anything.Requires oxidecomputer/idolatry#69 and staged on top of #2232