[v0.2.0 Refactor] [Type-C] Remove static_cell dependency and improve context handling #668
+1,318
−697
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tullom
force-pushed
the
type-c/remove-global-statics
branch
3 times, most recently
from
35a66ba to
ffb0e7e
Compare
…context handling - Removed `static_cell` dependency from Cargo.lock and Cargo.toml in examples and type-c-service. - Updated Type-C service methods to accept a reference to `intrusive_list::IntrusiveList` for better context management. - Modified various service methods to pass the controllers list where necessary, ensuring proper context usage. - Commented out unused code related to power policy channels and service initialization. - Adjusted the task closure to work with the updated service structure. - Enhanced error handling and logging throughout the service methods. - Initializing the service will now register all PD controllers before running the type-c task WIP
tullom
force-pushed
the
type-c/remove-global-statics
branch
from
8c975f6 to
16aadd1
Compare
tullom
requested review from
RobertZ2011,
asasine,
felipebalbi,
gjpmsft and
jerrysxie
RobertZ2011
approved these changes
Jan 12, 2026
kurtjd
approved these changes
Jan 12, 2026
asasine
approved these changes
Jan 14, 2026
asasine
left a comment
asasine
left a comment
There was a problem hiding this comment.
Looks functional so approved. All of my comments are therefore non-blocking.
&'static->&'ais a step for testability.- Adding methods to
Contextand getting rid of the external public static functions is a usability improvement. - Using
self.controllers()instead of passing a field as an arg to your own methods is hygeine and usability.
| /// Base storage | ||
| pub struct Storage<const N: usize, M: RawMutex> { | ||
| // Registration-related | ||
| context: &'static embedded_services::type_c::controller::Context, |
There was a problem hiding this comment.
Does this have to be &'static or can it be a lifetime parameter?
| async fn process_external_command(&self, command: &external::Command) -> external::Response<'static> { | ||
| async fn process_external_command( | ||
| &self, | ||
| controllers: &intrusive_list::IntrusiveList, |
There was a problem hiding this comment.
Service stores the reference to &'a IntrusiveList so this doesn't need to be an arg to this method (or others like it); you can just refer to self.controllers() within this type's impls.
| id: controller_id, | ||
| data: ControllerCommandData::Reset, | ||
| })) | ||
| pub async fn reset_controller(ctx: &Context, controller_id: ControllerId) -> Result<(), PdError> { |
There was a problem hiding this comment.
The signature of these functions looks a lot like it should be a method on Context. Since the caller needs the Context object, they can simply context.reset_controller(id) rather than external::reset_controller(context, id)
impl Context {
pub async fn reset_controller(&self, controller_id: ControllerId) -> Result<(), PdError> {
// --snip--
}
}| id: controller_id, | ||
| data: ControllerCommandData::Reset, | ||
| })) | ||
| pub async fn reset_controller(ctx: &Context, controller_id: ControllerId) -> Result<(), PdError> { |
There was a problem hiding this comment.
nit: ctx -> context here and elsewhere. Dont use abbreviations when not necessary.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
static_celldependency from Cargo.lock and Cargo.toml in examples and type-c-service.intrusive_list::IntrusiveListfor better context management.