Introduces a common trait for Copper Applications.#345
Conversation
This allows use some generalization to manage a set of copper application generated by the new missions feature.
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a common CuApplication and CuSimApplication trait to standardize application lifecycles, adjusts simulation callback types to remove explicit lifetimes, and updates the derive macros and prelude exports to support these new abstractions.
- Add
appmodule withCuApplicationandCuSimApplicationtraits incu29_runtime - Remove lifetime parameters from
CuTaskCallbackStateand update example closures - Update derive code to generate implementations for the new traits and re-export in prelude
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| examples/cu_rp_balancebot/src/sim.rs | Updated simulation callback closure to match new SimStep signature |
| examples/cu_rp_balancebot/src/resim.rs | Same closure signature adjustment for resimulation |
| examples/cu_rp_balancebot/Cargo.toml | Enabled macro_debug feature for the cu29 dependency |
| core/cu29_runtime/src/simulation.rs | Removed lifetime parameter from CuTaskCallbackState enum |
| core/cu29_runtime/src/lib.rs | Added new app module |
| core/cu29_runtime/src/app.rs | Introduced CuApplication and CuSimApplication traits |
| core/cu29_derive/src/lib.rs | Updated derive logic to generate new trait impls and SimStep use |
| core/cu29/src/lib.rs | Re-exported app traits in prelude |
Comments suppressed due to low confidence (3)
core/cu29_runtime/src/app.rs:15
- [nitpick] Consider adding unit or integration tests for
CuApplicationandCuSimApplicationto ensure all lifecycle methods (new,start_all_tasks,run_one_iteration, etc.) behave as expected.
pub trait CuApplication {
examples/cu_rp_balancebot/src/sim.rs:114
- The closure parameter type
default::SimSteprequires specifying its lifetime (e.g.,default::SimStep<'_>) or using a higher-rank trait (for<'a> default::SimStep<'a>). Omitting the lifetime will fail to compile.
let mut sim_callback = move |step: default::SimStep| -> SimOverride {
examples/cu_rp_balancebot/src/resim.rs:39
- As above,
default::SimStepis generic over a lifetime and must be written with a lifetime argument (e.g.,default::SimStep<'_>) or handled viafor<'a>.
let mut sim_callback = move |step: default::SimStep| -> SimOverride {
mik90
left a comment
There was a problem hiding this comment.
Minor comment on the ordering for calling the common API
| /// * `Ok(())` - If all tasks are started successfully. | ||
| /// * `Err(CuResult)` - If an error occurs while attempting to start one | ||
| /// or more tasks. | ||
| fn start_all_tasks( |
There was a problem hiding this comment.
Im guessing that this has to be called before either of the run*() functions, and stop() has to be called afterwards, is this right?
If so, it's possible that these could just error out if they're out of order, but it might be best to document the ordering and/or declare them in order that they can be executed (start, runs, stop) too.
There could be an RAII type that enforces this more strictly as well, or the run() calls could check for whether the tasks are already started but that doesn't solve the case of the stop.
Same thing applies to the normal application
There was a problem hiding this comment.
ha yes good idea we could build a StartedApplication etc ... Let me reorder that for now and let's add that in another PR
There was a problem hiding this comment.
Makes sense to me. Some StartedApplication that'd expose a .run() interface. Although that'd be 4 traits to impl since the sim and non-sim ones have separate implementations for running
There was a problem hiding this comment.
yeah and a bunch of blind proc macro changes to make lol ... not fun :)
There was a problem hiding this comment.
Yep, just a reorder seems best now that i think of the trait count
This allows to manage a set of copper applications generated by the new missions feature from a common tratir.