cfu service: deduping common response helper methods#773
cfu service: deduping common response helper methods#773sanjay-nagesh wants to merge 3 commits intoOpenDevicePartnership:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses Issue #388 by deduplicating CFU “common response” construction logic (invalid FW version + content rejection) into a shared helper module and updating existing CFU components to use it.
Changes:
- Added
cfu-service-internal response helper functions in a newresponsesmodule. - Updated
BufferandSplitterto call the shared helpers instead of maintaining duplicate local methods. - Wired the new module into the crate via
lib.rs.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
cfu-service/src/splitter.rs |
Replaces local helper methods with calls into crate::responses for common response creation. |
cfu-service/src/buffer.rs |
Replaces duplicated response-building helpers with shared crate::responses utilities. |
cfu-service/src/responses.rs |
Introduces shared helpers for invalid FW version responses and content rejection responses. |
cfu-service/src/lib.rs |
Adds the internal responses module to the crate. |
|
@sanjay-nagesh You'll need to rebase this branch to pull in the latest changes from main before it can be merged. |
1f41ccc to
e07e8c9
Compare
| let dev_inf = FwVerComponentInfo::new(FwVersion::new(INVALID_FW_VERSION), component_id); | ||
| let comp_info: [FwVerComponentInfo; MAX_CMPT_COUNT] = [dev_inf; MAX_CMPT_COUNT]; |
There was a problem hiding this comment.
The local variable name dev_inf is unclear/abbreviated; consider renaming to something more descriptive (e.g., dev_info or component_info_entry) to make the helper’s intent easier to read/maintain.
| let dev_inf = FwVerComponentInfo::new(FwVersion::new(INVALID_FW_VERSION), component_id); | |
| let comp_info: [FwVerComponentInfo; MAX_CMPT_COUNT] = [dev_inf; MAX_CMPT_COUNT]; | |
| let component_info_entry = FwVerComponentInfo::new(FwVersion::new(INVALID_FW_VERSION), component_id); | |
| let comp_info: [FwVerComponentInfo; MAX_CMPT_COUNT] = | |
| [component_info_entry; MAX_CMPT_COUNT]; |
| // Returns a content rejection response with the given block sequence number. | ||
| // This is used when firmware content cannot be delivered or handled correctly. | ||
| // | ||
| pub(crate) fn create_content_rejection(sequence: u16) -> InternalResponseData { |
There was a problem hiding this comment.
The standalone // line is unnecessary, and these look like function docs; consider converting them to rustdoc comments (/// ...) so they stay attached to the functions and match the style used elsewhere in this crate.
Fixed Issue #388