Feature/sc 123873/check hash #693
Conversation
| } | ||
| } else { | ||
| console.log(chalk.red(' ' + assetInfo.name + ' failed' + ' (hash should be ' + assetInfo.hash + ' but is not in the bundle)')); | ||
| console.log(chalk.red(' ' + appAsset.name + ' failed' + ' (hash should be ' + appAsset.hash + ' but is not in the bundle)')); | ||
| } |
There was a problem hiding this comment.
No functional change here. Felt like the readability of this code could be improved.
keeramis
requested review from
monkbroc,
hugomontero and
avtolstoy
and removed request for
monkbroc
src/cmd/binary.js
Outdated
| if (asset) { | ||
| const valid = isAssetValid(asset.data, assetInfo); | ||
| const valid = isAssetValid(asset.data, appAsset); | ||
| // isAssetValid(); |
There was a problem hiding this comment.
Reminder to remove commented out code
src/cmd/flash.js
Outdated
| await flashFiles({ device, flashSteps, ui: this.ui }); | ||
| } | ||
|
|
||
| async _filterAssetsOnDevice(device, modules) { | ||
| if (device.isInDfuMode) { | ||
| device = await usbUtils.reopenInNormalMode(device, { reset: true }); |
There was a problem hiding this comment.
Let's use a different approach because this makes flashing less reliable. If the application firmware is crashing and someone manually puts the device in DFU mode to reflash it and the CLI reopens it in normal mode, the device will crash before it can get the asset info. This will also call getAssetInfo even in case the application doesn't have assets.
I think we could create the flash steps as if we're flashing all the assets, but in flashFiles when it comes time to flash an asset, call getAssetInfo (cache the result in case there are other assets) and skip calling device.updateFirmware if the asset already exists.
| it ('returns the hash of the file', async () => { | ||
| const assetModules = await createAssetModules(); | ||
|
|
||
| const hash = await _get256Hash(assetModules[0]); |
There was a problem hiding this comment.
No need for await since _get256Hash and _skipAsset are not async (check for the other uses below too)
There was a problem hiding this comment.
Nice catch. I clicked the Merge button too fast. Fixed in #694
|
|
||
| it('skips asset is it is on the device', async () => { | ||
| const modules = await createAssetModules(); | ||
| const asset = modules.filter(m => m.filename === 'asset1.bin')[0]; |
There was a problem hiding this comment.
You can use find instead of filter with [0]
Description
Currently,
particle flash --localcommand flashes all the assets that are provided by the Particle project or the user app bundle. This PR skips flashing an asset which is already existing on the device. The reason for this change is to improve flashing times, which can be especially beneficial for large projects or when working with devices that have a significant amount of existing assets.This PR only makes changes to the
particle flash --localcommand and others are unchanged.How to Test
Setup:
particle-usbto this branch: Device::getAssetInfo() and new format in Device::getFirmwareModuleInfo() particle-usb#96device-osthis branch on the device: Asset OTA fixes device-os#2711Test:
particle bundle myApp.bin --assets /path/to/assetsparticle flash --local myApp.zip. Watch the progressmyApp.binandassetAare flashedparticle bundle myApp.bin --assets /path/to/assetsparticle flash --local myApp.zip. Watch the progressmyApp.binandassetBare flashed.assetAwill be skipped with a log message sayingSkipping asset assetA because it is already on the deviceRelated Issues / Discussions
SC: https://app.shortcut.com/particle/story/123873/flash-asset-only-if-it-s-not-existing-on-the-device
Protobuf PR: particle-iot/device-os-protobuf#22
Particle USB PR: particle-iot/particle-usb#96
Device OS PR: particle-iot/device-os#2711
Completeness