-
Notifications
You must be signed in to change notification settings - Fork 10
Adding eraseAppSettings to UpgradeOptions TypeScript type #545
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.
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
base: main
Are you sure you want to change the base?
Adding eraseAppSettings to UpgradeOptions TypeScript type #545
Conversation
@@ -114,6 +114,7 @@ class DeviceUpgrade { | |||
self.dfuManager = FirmwareUpgradeManager(transport: self.bleTransport!, delegate: self) | |||
let config = FirmwareUpgradeConfiguration( | |||
estimatedSwapTime: self.options.estimatedSwapTime, | |||
eraseAppSettings: self.options.eraseAppSettings, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will pass ts argument to the iOS FirmwareUpgradeConfiguration constructor
@@ -4,4 +4,5 @@ struct UpdateOptions: Record { | |||
@Field var estimatedSwapTime: TimeInterval = 0 | |||
@Field var upgradeFileType: Int = 0 | |||
@Field var upgradeMode: Int? | |||
@Field var eraseAppSettings: Bool = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We not passed, it will be false
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the PR.
Please could you smoke test (manually test) this using the example app on Android and iOS, checking that settings are preserved when the option is false, and settings are erased when the option is true?
@@ -28,6 +28,7 @@ class UpdateOptions : Record { | |||
@Field val estimatedSwapTime: Int = 0 | |||
@Field val upgradeFileType: Int = 0 | |||
@Field val upgradeMode: Int? = null | |||
@Field val eraseAppSettings: Boolean? = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please can you make sure this is passed through into FirmwareUpgradeManager
?
Sure, will do it and update the PR details with the results. |
Allowing TypeScript to pass
eraseAppSettings
as part ofUpgradeOptions
, so that we can have more control if we want to erase app settings or notCurrently, there is no way of erasing App settings as part of the MCU manager firmware update.
This is because this is handled by a boolean
eraseAppSettings
and its set tofalse
.By allowing it to pass it via
UpgradeOptions
, we can have more controlSelf Review:
Smoke Tests: