Skip to content

Refactor OtaUpdater::with_ota into OtaUpdater::ota_data to return partition data instead of a closure #4413

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Nov 10, 2025
Merged

Refactor OtaUpdater::with_ota into OtaUpdater::ota_data to return partition data instead of a closure #4413

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e89e7de
make OtaUpdater::with_ota public
JurajSadel Oct 30, 2025
d0e6405
changelog
JurajSadel Oct 30, 2025
9732686
refactor
JurajSadel Oct 30, 2025
b50042e
Apply suggestions from code review
JurajSadel Nov 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions esp-bootloader-esp-idf/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Added

- `OtaUpdater::ota_data()` (#4413)

### Changed

- `Ota::new()` now takes `FlashRegion` by value (#4413)

### Fixed

Expand Down
31 changes: 14 additions & 17 deletions esp-bootloader-esp-idf/src/ota.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ pub struct Ota<'a, F>
where
F: embedded_storage::Storage,
{
flash: &'a mut FlashRegion<'a, F>,
flash: FlashRegion<'a, F>,
ota_partition_count: usize,
}

Expand All @@ -165,10 +165,7 @@ where
/// doesn't represent a Data/Ota partition or the size is unexpected.
///
/// [Error::InvalidArgument] if the `ota_partition_count` exceeds the maximum or if it's 0.
pub fn new(
flash: &'a mut FlashRegion<'a, F>,
ota_partition_count: usize,
) -> Result<Ota<'a, F>, Error> {
pub fn new(flash: FlashRegion<'a, F>, ota_partition_count: usize) -> Result<Ota<'a, F>, Error> {
if ota_partition_count == 0 || ota_partition_count > 16 {
return Err(Error::InvalidArgument);
}
Expand Down Expand Up @@ -432,12 +429,12 @@ mod tests {
data: [0xff; 0x2000],
};

let mut mock_region = FlashRegion {
let mock_region = FlashRegion {
raw: mock_entry,
flash: &mut mock_flash,
};

let mut sut = Ota::new(&mut mock_region, 2).unwrap();
let mut sut = Ota::new(mock_region, 2).unwrap();
assert_eq!(
sut.current_app_partition().unwrap(),
AppPartitionSubType::Factory
Expand Down Expand Up @@ -482,12 +479,12 @@ mod tests {
mock_flash.data[0x0000..][..0x20].copy_from_slice(SLOT_COUNT_1_VALID);
mock_flash.data[0x1000..][..0x20].copy_from_slice(SLOT_INITIAL);

let mut mock_region = FlashRegion {
let mock_region = FlashRegion {
raw: mock_entry,
flash: &mut mock_flash,
};

let mut sut = Ota::new(&mut mock_region, 2).unwrap();
let mut sut = Ota::new(mock_region, 2).unwrap();
assert_eq!(
sut.current_app_partition().unwrap(),
AppPartitionSubType::Ota0
Expand Down Expand Up @@ -522,12 +519,12 @@ mod tests {
mock_flash.data[0x0000..][..0x20].copy_from_slice(SLOT_COUNT_1_VALID);
mock_flash.data[0x1000..][..0x20].copy_from_slice(SLOT_COUNT_2_NEW);

let mut mock_region = FlashRegion {
let mock_region = FlashRegion {
raw: mock_entry,
flash: &mut mock_flash,
};

let mut sut = Ota::new(&mut mock_region, 2).unwrap();
let mut sut = Ota::new(mock_region, 2).unwrap();
assert_eq!(
sut.current_app_partition().unwrap(),
AppPartitionSubType::Ota1
Expand Down Expand Up @@ -560,12 +557,12 @@ mod tests {
data: [0xff; 0x2000],
};

let mut mock_region = FlashRegion {
let mock_region = FlashRegion {
raw: mock_entry,
flash: &mut mock_flash,
};

let mut sut = Ota::new(&mut mock_region, 2).unwrap();
let mut sut = Ota::new(mock_region, 2).unwrap();
assert_eq!(
sut.current_app_partition().unwrap(),
AppPartitionSubType::Factory
Expand Down Expand Up @@ -626,12 +623,12 @@ mod tests {
data: [0xff; 0x2000],
};

let mut mock_region = FlashRegion {
let mock_region = FlashRegion {
raw: mock_entry,
flash: &mut mock_flash,
};

let mut sut = Ota::new(&mut mock_region, 4).unwrap();
let mut sut = Ota::new(mock_region, 4).unwrap();
assert_eq!(
sut.current_app_partition().unwrap(),
AppPartitionSubType::Factory
Expand Down Expand Up @@ -711,12 +708,12 @@ mod tests {
data: [0xff; 0x2000],
};

let mut mock_region = FlashRegion {
let mock_region = FlashRegion {
raw: mock_entry,
flash: &mut mock_flash,
};

let mut sut = Ota::new(&mut mock_region, 16).unwrap();
let mut sut = Ota::new(mock_region, 16).unwrap();
assert_eq!(
sut.current_app_partition().unwrap(),
AppPartitionSubType::Factory
Expand Down
23 changes: 12 additions & 11 deletions esp-bootloader-esp-idf/src/ota_updater.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,19 +68,20 @@ where
})
}

fn with_ota<R>(
&mut self,
f: impl FnOnce(crate::ota::Ota<'_, F>) -> Result<R, Error>,
) -> Result<R, Error> {
/// Returns an [`Ota`] for accessing the OTA-data partition.
///
/// # Errors
/// [Error::Invalid] if no OTA data partition was found.
pub fn ota_data(&mut self) -> Result<crate::ota::Ota<'_, F>, Error> {
let ota_part = self
.pt
.find_partition(crate::partitions::PartitionType::Data(
crate::partitions::DataPartitionSubType::Ota,
))?;
if let Some(ota_part) = ota_part {
let mut ota_part = ota_part.as_embedded_storage(self.flash);
let ota = crate::ota::Ota::new(&mut ota_part, self.ota_count)?;
f(ota)
let ota_part = ota_part.as_embedded_storage(self.flash);
let ota = crate::ota::Ota::new(ota_part, self.ota_count)?;
Ok(ota)
} else {
Err(Error::Invalid)
}
Expand Down Expand Up @@ -114,23 +115,23 @@ where

/// Returns the currently selected app partition.
pub fn selected_partition(&mut self) -> Result<crate::partitions::AppPartitionSubType, Error> {
self.with_ota(|mut ota| ota.current_app_partition())
self.ota_data()?.current_app_partition()
}

/// Get the [OtaImageState] of the currently selected partition.
///
/// # Errors
/// A [Error::InvalidState] if no partition is currently selected.
pub fn current_ota_state(&mut self) -> Result<OtaImageState, Error> {
self.with_ota(|mut ota| ota.current_ota_state())
self.ota_data()?.current_ota_state()
}

/// Set the [OtaImageState] of the currently selected slot.
///
/// # Errors
/// A [Error::InvalidState] if no partition is currently selected.
pub fn set_current_ota_state(&mut self, state: OtaImageState) -> Result<(), Error> {
self.with_ota(|mut ota| ota.set_current_ota_state(state))
self.ota_data()?.set_current_ota_state(state)
}

/// Selects the next active OTA-slot as current.
Expand All @@ -139,7 +140,7 @@ where
/// activated partition.
pub fn activate_next_partition(&mut self) -> Result<(), Error> {
let next_slot = self.next_ota_part()?;
self.with_ota(|mut ota| ota.set_current_app_partition(next_slot))
self.ota_data()?.set_current_app_partition(next_slot)
}

/// Returns a [FlashRegion] along with the [AppPartitionSubType] for the
Expand Down
Loading