From dc1511c85f75e3a8f86196dd45ecfc7a2f9ed198 Mon Sep 17 00:00:00 2001 From: keeramis Date: Fri, 26 Jul 2024 12:05:47 -0700 Subject: [PATCH 1/4] Obtain device state for dfu devices more accurately --- src/dfu.js | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/src/dfu.js b/src/dfu.js index db5fc5f..de775c0 100644 --- a/src/dfu.js +++ b/src/dfu.js @@ -236,18 +236,27 @@ class Dfu { * @return {Promise} Object with property 'protected' */ async getProtectionState() { - // setting 0 is for Internal Flash - await this.setAltSetting(0); - - let allSegmentsProtected = true; - this._memoryInfo.segments.forEach(s => { - if (!(s.erasable === true && s.writable === false && s.readable === false)) { - allSegmentsProtected = false; + try { + const res = await this._getStringDescriptor(0xfe); + const state = res.split(';').find(kv => kv.startsWith('s='))?.split('=')[1]; + + switch (state) { + case 'o': return { protected: false, overridden: false }; + case 'p': return { protected: true }; + case 's': return { protected: false, overridden: true }; + default: throw new Error('Unknown device state'); } - }); - // FIXME: Currently, device-os does not reliably distinguish the `overridden` value for different protection modes. - // As a workaround, we use `null` to uniquely indicate the distinction. - return { protected: allSegmentsProtected, overridden: null }; + } catch (error) { + // Fallback for devices with Device-OS < 6.1.2 + await this.setAltSetting(0); // setting 0 is for Internal Flash + + const allSegmentsProtected = this._memoryInfo.segments.every(s => + s.erasable === true && s.writable === false && s.readable === false + ); + + // Use `null` for `overridden` to indicate older Device-OS version + return { protected: allSegmentsProtected, overridden: null }; + } } /** From 851d12e884d4e7226afdd95677d596aea257ae3d Mon Sep 17 00:00:00 2001 From: keeramis Date: Fri, 26 Jul 2024 12:16:42 -0700 Subject: [PATCH 2/4] [tests] add tests --- src/dfu.js | 2 +- src/dfu.test.js | 30 ++++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/dfu.js b/src/dfu.js index de775c0..f58a305 100644 --- a/src/dfu.js +++ b/src/dfu.js @@ -238,7 +238,7 @@ class Dfu { async getProtectionState() { try { const res = await this._getStringDescriptor(0xfe); - const state = res.split(';').find(kv => kv.startsWith('s='))?.split('=')[1]; + const state = res.split(';').find(kv => kv.startsWith('sm='))?.split('=')[1]; switch (state) { case 'o': return { protected: false, overridden: false }; diff --git a/src/dfu.test.js b/src/dfu.test.js index 1123b7d..efeb3a4 100644 --- a/src/dfu.test.js +++ b/src/dfu.test.js @@ -365,8 +365,35 @@ describe('dfu', () => { }); describe('getProtectionState', () => { + it ('detects an Open Device from string desc for Device-OS >= 6.1.2', async () => { + const dfu = new Dfu(); + sinon.stub(dfu, '_getStringDescriptor').resolves("sm=o"); + + const res = await dfu.getProtectionState(); + expect(res.protected).to.eql(false); + expect(res.overridden).to.eql(false); + }); + + it ('detects a Protected Device from string desc for Device-OS >= 6.1.2', async () => { + const dfu = new Dfu(); + sinon.stub(dfu, '_getStringDescriptor').resolves("sm=p"); + + const res = await dfu.getProtectionState(); + expect(res.protected).to.eql(true); + }); + + it ('detects a Protected Device in Service Mode from string desc for Device-OS >= 6.1.2', async () => { + const dfu = new Dfu(); + sinon.stub(dfu, '_getStringDescriptor').resolves("sm=s"); + + const res = await dfu.getProtectionState(); + expect(res.protected).to.eql(false); + expect(res.overridden).to.eql(true); + }); + it('returns that all segments are protected', async () => { const dfu = new Dfu(); + sinon.stub(dfu, '_getStringDescriptor').rejects('random error'); sinon.stub(dfu, 'setAltSetting').resolves(); const internalFlashDesc = { 'name': 'Internal Flash', @@ -409,11 +436,13 @@ describe('dfu', () => { const res = await dfu.getProtectionState(); + expect(dfu.setAltSetting).to.have.been.calledOnce; expect(res.protected).to.eql(true); }); it('returns that all segments are not protected', async () => { const dfu = new Dfu(); + sinon.stub(dfu, '_getStringDescriptor').rejects('random error'); sinon.stub(dfu, 'setAltSetting').resolves(); const internalFlashDesc = { 'name': 'Internal Flash', @@ -456,6 +485,7 @@ describe('dfu', () => { const res = await dfu.getProtectionState(); + expect(dfu.setAltSetting).to.have.been.calledOnce; expect(res.protected).to.eql(false); }); }); From 827ada4ffb6f6e281be67654ea0be6cd4765b01e Mon Sep 17 00:00:00 2001 From: keeramis Date: Fri, 26 Jul 2024 12:40:15 -0700 Subject: [PATCH 3/4] minor --- src/dfu.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/dfu.js b/src/dfu.js index f58a305..5ea7ef9 100644 --- a/src/dfu.js +++ b/src/dfu.js @@ -237,9 +237,8 @@ class Dfu { */ async getProtectionState() { try { - const res = await this._getStringDescriptor(0xfe); - const state = res.split(';').find(kv => kv.startsWith('sm='))?.split('=')[1]; - + const res = await this._getStringDescriptor(0xfa); + const state = res.split(';').find(kv => kv.startsWith('sm='))?.split('=')[1]?.trim().charAt(0); switch (state) { case 'o': return { protected: false, overridden: false }; case 'p': return { protected: true }; @@ -247,6 +246,7 @@ class Dfu { default: throw new Error('Unknown device state'); } } catch (error) { + console.warn('Failed to get protection state', error); // Fallback for devices with Device-OS < 6.1.2 await this.setAltSetting(0); // setting 0 is for Internal Flash From 530b91a7a436cfe9c837e40a6a6f9b193137b60e Mon Sep 17 00:00:00 2001 From: keeramis Date: Fri, 26 Jul 2024 12:56:20 -0700 Subject: [PATCH 4/4] Lint fix --- src/dfu.js | 11 +++++------ src/dfu.test.js | 6 +++--- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/dfu.js b/src/dfu.js index 5ea7ef9..2954f6f 100644 --- a/src/dfu.js +++ b/src/dfu.js @@ -238,7 +238,7 @@ class Dfu { async getProtectionState() { try { const res = await this._getStringDescriptor(0xfa); - const state = res.split(';').find(kv => kv.startsWith('sm='))?.split('=')[1]?.trim().charAt(0); + const state = res.split(';').find(kv => kv.startsWith('sm=')).split('=')[1].trim().charAt(0); switch (state) { case 'o': return { protected: false, overridden: false }; case 'p': return { protected: true }; @@ -246,15 +246,14 @@ class Dfu { default: throw new Error('Unknown device state'); } } catch (error) { - console.warn('Failed to get protection state', error); // Fallback for devices with Device-OS < 6.1.2 await this.setAltSetting(0); // setting 0 is for Internal Flash - - const allSegmentsProtected = this._memoryInfo.segments.every(s => + + const allSegmentsProtected = this._memoryInfo.segments.every(s => s.erasable === true && s.writable === false && s.readable === false ); - - // Use `null` for `overridden` to indicate older Device-OS version + + // Use `null` for `overridden` since we cannot distinguish between Open and Service Mode return { protected: allSegmentsProtected, overridden: null }; } } diff --git a/src/dfu.test.js b/src/dfu.test.js index efeb3a4..4e9f3b0 100644 --- a/src/dfu.test.js +++ b/src/dfu.test.js @@ -367,7 +367,7 @@ describe('dfu', () => { describe('getProtectionState', () => { it ('detects an Open Device from string desc for Device-OS >= 6.1.2', async () => { const dfu = new Dfu(); - sinon.stub(dfu, '_getStringDescriptor').resolves("sm=o"); + sinon.stub(dfu, '_getStringDescriptor').resolves('sm=o'); const res = await dfu.getProtectionState(); expect(res.protected).to.eql(false); @@ -376,7 +376,7 @@ describe('dfu', () => { it ('detects a Protected Device from string desc for Device-OS >= 6.1.2', async () => { const dfu = new Dfu(); - sinon.stub(dfu, '_getStringDescriptor').resolves("sm=p"); + sinon.stub(dfu, '_getStringDescriptor').resolves('sm=p'); const res = await dfu.getProtectionState(); expect(res.protected).to.eql(true); @@ -384,7 +384,7 @@ describe('dfu', () => { it ('detects a Protected Device in Service Mode from string desc for Device-OS >= 6.1.2', async () => { const dfu = new Dfu(); - sinon.stub(dfu, '_getStringDescriptor').resolves("sm=s"); + sinon.stub(dfu, '_getStringDescriptor').resolves('sm=s'); const res = await dfu.getProtectionState(); expect(res.protected).to.eql(false);