Skip to content

Add get_app_at_slot method to retrieve descriptor for any index#9

Merged
jerrysxie merged 1 commit intoOpenDevicePartnership:mainfrom
Ctru14:connortruono/get_app_slot
Mar 25, 2025
Merged

Add get_app_at_slot method to retrieve descriptor for any index#9
jerrysxie merged 1 commit intoOpenDevicePartnership:mainfrom
Ctru14:connortruono/get_app_slot

Conversation

@Ctru14
Copy link

@Ctru14 Ctru14 commented Mar 25, 2025

For the BootableRegionDescriptors object to return the AppImageDescriptor from an app at any specified index. The new get_app_at_slot(idx) method will verify the index is within the number of app slots then return the specified app descriptor.

JamesHuard
JamesHuard previously approved these changes Mar 25, 2025
@Ctru14 Ctru14 changed the title Add get_app_slot method to retrieve descriptor for any index Add get_app_at_slot method to retrieve descriptor for any index Mar 25, 2025
@jerrysxie jerrysxie added the enhancement New feature or request label Mar 25, 2025
@felipebalbi felipebalbi moved this to In review in Embedded Controller Mar 25, 2025
Copy link

@felipebalbi felipebalbi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rebase on current main and make sure to combine commits such that each commit in your PR compiles and runs without known issues.

@Ctru14 Ctru14 force-pushed the connortruono/get_app_slot branch from 1868ee7 to 1892b3b Compare March 25, 2025 17:13
JamesHuard
JamesHuard previously approved these changes Mar 25, 2025
@Ctru14 Ctru14 requested a review from felipebalbi March 25, 2025 17:15
@jerrysxie jerrysxie requested a review from Copilot March 25, 2025 17:18
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request adds a new method to the BootableRegionDescriptors struct to allow retrieval of a descriptor for a specified app slot while ensuring the provided index is valid. It also updates the Rust minimal supported version (msrv) in the workflow configuration.

  • Added the get_app_at_slot method in src/lib.rs to retrieve an app descriptor at a given index.
  • Updated the msrv from 1.79 to 1.84 in the GitHub Actions workflow.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/lib.rs Added the get_app_at_slot method with an index bounds check and call to from_region.
.github/workflows/check.yml Updated the msrv version to support new namespaced features.
Comments suppressed due to low confidence (1)

src/lib.rs:192

  • [nitpick] Consider renaming the parameter 'app_index' to 'app_slot' for consistency with other code references such as the 'app_slot' parameter in from_region and 'num_app_slots' in the header.
pub fn get_app_at_slot(&self, app_index: u32) -> Result<AppImageDescriptor, ParseError> {

Copy link

@felipebalbi felipebalbi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not all commits compile. Additionally, you still have a few clippy errors.

slot

Reword parameter name to app_slot

Fix app_slot reference
@Ctru14 Ctru14 force-pushed the connortruono/get_app_slot branch from 69f9af3 to da186e8 Compare March 25, 2025 17:33
@jerrysxie jerrysxie merged commit cee1877 into OpenDevicePartnership:main Mar 25, 2025
9 checks passed
@github-project-automation github-project-automation bot moved this from In review to Done in Embedded Controller Mar 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants