Refactoring to test the double firmware update procedure #234

(while performing the same, known to be working firmware update for Mi1A)

Result: double firmware update procedure works on Mi1A.

Also updated FirmwareTest. Perform all tests not only in the test itself,
but also at runtime before doing the actual update.

Further:
- fix setting of firmwareInfoSent state variable, which prevented installation
  of the section firmware
- make one string translatable

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/miband/AbstractMiFirmwareInfo.java

@@ -29,6 +29,13 @@ public abstract class AbstractMiFirmwareInfo {
     }
 
     private static AbstractMiFirmwareInfo[] getFirmwareInfoCandidatesFor(byte[] wholeFirmwareBytes) {
+        if (MiBandSupport.MI_1A_HR_FW_UPDATE_TEST_MODE_ENABLED) {
+            TestMi1AFirmwareInfo info = TestMi1AFirmwareInfo.getInstance(wholeFirmwareBytes);
+            if (info != null) {
+                return new AbstractMiFirmwareInfo[] { info };
+            }
+        }
+
         AbstractMiFirmwareInfo[] candidates = new AbstractMiFirmwareInfo[3];
         int i = 0;
         Mi1FirmwareInfo mi1Info = Mi1FirmwareInfo.getInstance(wholeFirmwareBytes);

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/miband/CompositeMiFirmwareInfo.java

@@ -0,0 +1,96 @@
+package nodomain.freeyourgadget.gadgetbridge.service.devices.miband;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Abstract Mi firmware info class with two child info instances.
+ */
+public abstract class CompositeMiFirmwareInfo<T extends AbstractMiFirmwareInfo> extends AbstractMiFirmwareInfo {
+    private static final Logger LOG = LoggerFactory.getLogger(CompositeMiFirmwareInfo.class);
+
+    private final T fw1Info;
+    private final T fw2Info;
+
+    protected CompositeMiFirmwareInfo(byte[] wholeFirmwareBytes, T info1, T info2) {
+        super(wholeFirmwareBytes);
+        fw1Info = info1;
+        fw2Info = info2;
+    }
+
+    @Override
+    public void checkValid() throws IllegalArgumentException {
+        super.checkValid();
+
+        if (getFirst().getFirmwareOffset() == getSecond().getFirmwareOffset()) {
+            throw new IllegalArgumentException("Illegal firmware offsets: " + getLengthsOffsetsString());
+        }
+        if (getFirst().getFirmwareOffset() < 0 || getSecond().getFirmwareOffset() < 0
+                || getFirst().getFirmwareLength() <= 0 || getSecond().getFirmwareLength() <= 0) {
+            throw new IllegalArgumentException("Illegal firmware offsets/lengths: " + getLengthsOffsetsString());
+        }
+
+        int firstEndIndex = getFirst().getFirmwareOffset() + getFirst().getFirmwareLength();
+        if (getSecond().getFirmwareOffset() < firstEndIndex) {
+            throw new IllegalArgumentException("Invalid firmware, second fw starts before first fw ends: " + firstEndIndex + "," + getSecond().getFirmwareOffset());
+        }
+        int secondEndIndex = getSecond().getFirmwareOffset();
+        if (wholeFirmwareBytes.length < firstEndIndex || wholeFirmwareBytes.length < secondEndIndex) {
+            throw new IllegalArgumentException("Invalid firmware size, or invalid offsets/lengths: " + getLengthsOffsetsString());
+        }
+
+        getFirst().checkValid();
+        getSecond().checkValid();
+    }
+
+    protected String getLengthsOffsetsString() {
+        return getFirst().getFirmwareOffset() + "," + getFirst().getFirmwareLength()
+                + "; "
+                + getSecond().getFirmwareOffset() + "," + getSecond().getFirmwareLength();
+    }
+
+    @Override
+    public T getFirst() {
+        return fw1Info;
+    }
+
+    @Override
+    public T getSecond() {
+        return fw2Info;
+    }
+
+    @Override
+    protected boolean isGenerallySupportedFirmware() {
+        try {
+            if (!isHeaderValid()) {
+                LOG.info("unrecognized header");
+                return false;
+            }
+            return fw1Info.isGenerallySupportedFirmware()
+                    && fw2Info.isGenerallySupportedFirmware()
+                    && fw1Info.getFirmwareBytes().length > 0
+                    && fw2Info.getFirmwareBytes().length > 0;
+        } catch (IndexOutOfBoundsException ex) {
+            LOG.warn("invalid firmware or bug: " + ex.getLocalizedMessage(), ex);
+            return false;
+        } catch (IllegalArgumentException ex) {
+            LOG.warn("not supported 1S firmware: " + ex.getLocalizedMessage(), ex);
+            return false;
+        }
+    }
+
+    @Override
+    public int getFirmwareOffset() {
+        throw new UnsupportedOperationException("call this method on getFirmwareXInfo()");
+    }
+
+    @Override
+    public int getFirmwareLength() {
+        throw new UnsupportedOperationException("call this method on getFirmwareXInfo()");
+    }
+
+    @Override
+    public int getFirmwareVersion() {
+        throw new UnsupportedOperationException("call this method on getFirmwareXInfo()");
+    }
+}

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/miband/DeviceInfo.java

@@ -17,6 +17,7 @@ public class DeviceInfo extends AbstractInfo {
      * Heart rate firmware version identifier
      */
     public final int fw2Version;
+    private boolean test1AHRMode;
 
 
     private boolean isChecksumCorrect(byte[] data) {
@@ -71,11 +72,18 @@ public class DeviceInfo extends AbstractInfo {
     }
 
     public int getHeartrateFirmwareVersion() {
+        if (test1AHRMode) {
+            return fwVersion;
+        }
         return fw2Version;
     }
 
+    public void setTest1AHRMode(boolean enableTestMode) {
+        test1AHRMode = enableTestMode;
+    }
+
     public boolean supportsHeartrate() {
-        return isMili1S();
+        return isMili1S() || (test1AHRMode && isMili1A());
     }
 
     @Override

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/miband/Mi1SFirmwareInfo.java

@@ -5,13 +5,15 @@ import android.support.annotation.Nullable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import nodomain.freeyourgadget.gadgetbridge.devices.miband.MiBandConst;
+import nodomain.freeyourgadget.gadgetbridge.impl.GBDevice;
 import nodomain.freeyourgadget.gadgetbridge.util.ArrayUtils;
 
 /**
  * FW1 is Mi Band firmware
  * FW2 is heartrate firmware
  */
-public class Mi1SFirmwareInfo extends AbstractMi1SFirmwareInfo {
+public class Mi1SFirmwareInfo extends CompositeMiFirmwareInfo {
     private static final Logger LOG = LoggerFactory.getLogger(Mi1SFirmwareInfo.class);
 
     private static final byte[] DOUBLE_FW_HEADER = new byte[] {
@@ -22,13 +24,18 @@ public class Mi1SFirmwareInfo extends AbstractMi1SFirmwareInfo {
     };
     private static final int DOUBLE_FW_HEADER_OFFSET = 0;
 
-    private final Mi1SFirmwareInfoFW1 fw1Info;
-    private final Mi1SFirmwareInfoFW2 fw2Info;
-
     private Mi1SFirmwareInfo(byte[] wholeFirmwareBytes) {
-        super(wholeFirmwareBytes);
+        super(wholeFirmwareBytes, new Mi1SFirmwareInfoFW1(wholeFirmwareBytes), new Mi1SFirmwareInfoFW2(wholeFirmwareBytes));
-        fw1Info = new Mi1SFirmwareInfoFW1(wholeFirmwareBytes);
+    }
-        fw2Info = new Mi1SFirmwareInfoFW2(wholeFirmwareBytes);
+
+    @Override
+    public boolean isGenerallyCompatibleWith(GBDevice device) {
+        return MiBandConst.MI_1S.equals(device.getHardwareVersion());
+    }
+
+    @Override
+    public boolean isSingleMiBandFirmware() {
+        return false;
     }
 
     protected boolean isHeaderValid() {
@@ -36,41 +43,8 @@ public class Mi1SFirmwareInfo extends AbstractMi1SFirmwareInfo {
         return ArrayUtils.equals(DOUBLE_FW_HEADER, wholeFirmwareBytes, DOUBLE_FW_HEADER_OFFSET, DOUBLE_FW_HEADER_OFFSET + DOUBLE_FW_HEADER.length);
     }
 
-    @Override
-    public void checkValid() throws IllegalArgumentException {
-        super.checkValid();
-        int firstEndIndex = getFirst().getFirmwareOffset() + getFirst().getFirmwareLength();
-        if (getSecond().getFirmwareOffset() < firstEndIndex) {
-            throw new IllegalArgumentException("Invalid firmware offsets/lengths: " + getLengthsOffsetsString());
-        }
-        int secondEndIndex = getSecond().getFirmwareOffset();
-        if (wholeFirmwareBytes.length < firstEndIndex || wholeFirmwareBytes.length < secondEndIndex) {
-            throw new IllegalArgumentException("Invalid firmware size, or invalid offsets/lengths: " + getLengthsOffsetsString());
-        }
-        if (getSecond().getFirmwareOffset() < firstEndIndex) {
-            throw new IllegalArgumentException("Invalid firmware, second fw starts before first fw ends: " + firstEndIndex + "," + getSecond().getFirmwareOffset());
-        }
-    }
-
-    protected String getLengthsOffsetsString() {
-        return getFirst().getFirmwareOffset() + "," + getFirst().getFirmwareLength()
-                + "; "
-                + getSecond().getFirmwareOffset() + "," + getSecond().getFirmwareLength();
-    }
-
-    @Override
-    public AbstractMiFirmwareInfo getFirst() {
-        return fw1Info;
-    }
-
-    @Override
-    public AbstractMiFirmwareInfo getSecond() {
-        return fw2Info;
-    }
-
-    public static
     @Nullable
-    Mi1SFirmwareInfo getInstance(byte[] wholeFirmwareBytes) {
+    public static Mi1SFirmwareInfo getInstance(byte[] wholeFirmwareBytes) {
         Mi1SFirmwareInfo info = new Mi1SFirmwareInfo(wholeFirmwareBytes);
         if (info.isGenerallySupportedFirmware()) {
             return info;
@@ -78,39 +52,4 @@ public class Mi1SFirmwareInfo extends AbstractMi1SFirmwareInfo {
         LOG.info("firmware not supported");
         return null;
     }
-
-    @Override
-    protected boolean isGenerallySupportedFirmware() {
-        try {
-            if (!isHeaderValid()) {
-                LOG.info("unrecognized header");
-                return false;
-            }
-            return fw1Info.isGenerallySupportedFirmware()
-                    && fw2Info.isGenerallySupportedFirmware()
-                    && fw1Info.getFirmwareBytes().length > 0
-                    && fw2Info.getFirmwareBytes().length > 0;
-        } catch (IndexOutOfBoundsException ex) {
-            LOG.warn("invalid firmware or bug: " + ex.getLocalizedMessage(), ex);
-            return false;
-        } catch (IllegalArgumentException ex) {
-            LOG.warn("not supported 1S firmware: " + ex.getLocalizedMessage(), ex);
-            return false;
-        }
-    }
-
-    @Override
-    public int getFirmwareOffset() {
-        throw new UnsupportedOperationException("call this method on getFirmwareXInfo()");
-    }
-
-    @Override
-    public int getFirmwareLength() {
-        throw new UnsupportedOperationException("call this method on getFirmwareXInfo()");
-    }
-
-    @Override
-    public int getFirmwareVersion() {
-        throw new UnsupportedOperationException("call this method on getFirmwareXInfo()");
-    }
 }

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/miband/MiBandSupport.java

@@ -77,6 +77,11 @@ import static nodomain.freeyourgadget.gadgetbridge.devices.miband.MiBandConst.ge
 public class MiBandSupport extends AbstractBTLEDeviceSupport {
 
     private static final Logger LOG = LoggerFactory.getLogger(MiBandSupport.class);
+    /**
+     * This is just for temporary testing of Mi1A double firmware update.
+     * DO NOT SET TO TRUE UNLESS YOU KNOW WHAT YOU'RE DOING!
+     */
+    public static final boolean MI_1A_HR_FW_UPDATE_TEST_MODE_ENABLED = false;
     private volatile boolean telephoneRinging;
     private volatile boolean isLocatingDevice;
 
@@ -833,6 +838,7 @@ public class MiBandSupport extends AbstractBTLEDeviceSupport {
     private void handleDeviceInfo(byte[] value, int status) {
         if (status == BluetoothGatt.GATT_SUCCESS) {
             mDeviceInfo = new DeviceInfo(value);
+            mDeviceInfo.setTest1AHRMode(MI_1A_HR_FW_UPDATE_TEST_MODE_ENABLED);
             if (getDeviceInfo().supportsHeartrate()) {
                 getDevice().addDeviceInfo(new GenericItem(
                         getContext().getString(R.string.DEVINFO_HR_VER),

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/miband/TestMi1AFirmwareInfo.java

@@ -0,0 +1,76 @@
+package nodomain.freeyourgadget.gadgetbridge.service.devices.miband;
+
+import android.support.annotation.Nullable;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import nodomain.freeyourgadget.gadgetbridge.impl.GBDevice;
+
+/**
+ * This is a class just for testing the dual fw firmware update procedure.
+ * It uses two instances of the known-to-be-working Mi1A firmware update instances
+ * and combines them in a CompositeMiFirmwareInfo.
+ *
+ * Most methods simply delegate to one of the child instances (FW1).
+ *
+ * FW1 is the default Mi 1A Band firmware
+ * FW2 is the same default Mi 1A Band firmware
+ */
+public class TestMi1AFirmwareInfo extends CompositeMiFirmwareInfo {
+    private static final Logger LOG = LoggerFactory.getLogger(TestMi1AFirmwareInfo.class);
+
+    private TestMi1AFirmwareInfo(byte[] wholeFirmwareBytes) {
+        super(wholeFirmwareBytes, new Mi1AFirmwareInfo(wholeFirmwareBytes), new Mi1AFirmwareInfo(wholeFirmwareBytes));
+    }
+
+    @Override
+    public void checkValid() throws IllegalArgumentException {
+//        super.checkValid();
+// unfortunately we cannot use all of the checks in the superclass, so we roll our own
+
+        if (getFirst().getFirmwareOffset() != getSecond().getFirmwareOffset()) {
+            throw new IllegalArgumentException("Test firmware offsets should be the same: " + getLengthsOffsetsString());
+        }
+        if (getFirst().getFirmwareOffset() < 0 || getSecond().getFirmwareOffset() < 0
+                || getFirst().getFirmwareLength() <= 0 || getSecond().getFirmwareLength() <= 0) {
+            throw new IllegalArgumentException("Illegal test firmware offsets/lengths: " + getLengthsOffsetsString());
+        }
+
+        if (getFirst().getFirmwareLength() != getSecond().getFirmwareLength()) {
+            throw new IllegalArgumentException("Illegal test firmware lengths: " + getLengthsOffsetsString());
+        }
+        int firstEndIndex = getFirst().getFirmwareOffset() + getFirst().getFirmwareLength();
+        int secondEndIndex = getSecond().getFirmwareOffset();
+        if (wholeFirmwareBytes.length < firstEndIndex || wholeFirmwareBytes.length < secondEndIndex) {
+            throw new IllegalArgumentException("Invalid test firmware size, or invalid test offsets/lengths: " + getLengthsOffsetsString());
+        }
+
+        getFirst().checkValid();
+        getSecond().checkValid();
+    }
+
+    @Override
+    public boolean isGenerallyCompatibleWith(GBDevice device) {
+        return getFirst().isGenerallyCompatibleWith(device);
+    }
+
+    @Override
+    public boolean isSingleMiBandFirmware() {
+        return false;
+    }
+
+    protected boolean isHeaderValid() {
+        return getFirst().isHeaderValid();
+    }
+
+    @Nullable
+    public static TestMi1AFirmwareInfo getInstance(byte[] wholeFirmwareBytes) {
+        TestMi1AFirmwareInfo info = new TestMi1AFirmwareInfo(wholeFirmwareBytes);
+        if (info.isGenerallySupportedFirmware()) {
+            return info;
+        }
+        LOG.info("firmware not supported");
+        return null;
+    }
+}

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/miband/operations/UpdateFirmwareOperation.java

@@ -15,7 +15,9 @@ import java.util.UUID;
 import nodomain.freeyourgadget.gadgetbridge.R;
 import nodomain.freeyourgadget.gadgetbridge.devices.miband.MiBandFWHelper;
 import nodomain.freeyourgadget.gadgetbridge.devices.miband.MiBandService;
+import nodomain.freeyourgadget.gadgetbridge.service.btle.BtLEAction;
 import nodomain.freeyourgadget.gadgetbridge.service.btle.TransactionBuilder;
+import nodomain.freeyourgadget.gadgetbridge.service.btle.actions.PlainAction;
 import nodomain.freeyourgadget.gadgetbridge.service.btle.actions.SetDeviceBusyAction;
 import nodomain.freeyourgadget.gadgetbridge.service.btle.actions.SetProgressAction;
 import nodomain.freeyourgadget.gadgetbridge.service.devices.miband.AbstractMiFirmwareInfo;
@@ -52,8 +54,7 @@ public class UpdateFirmwareOperation extends AbstractMiBandOperation {
 
         updateCoordinator.initNextOperation();
 //        updateCoordinator.initNextOperation(); // FIXME: remove, just testing mi band 1s fw update
-        firmwareInfoSent = updateCoordinator.sendFwInfo();
+        if (!updateCoordinator.sendFwInfo()) {
-        if (!firmwareInfoSent) {
             GB.toast(getContext(), "Error sending firmware info, aborting.", Toast.LENGTH_LONG, GB.ERROR);
             done();
         }
@@ -282,7 +283,7 @@ public class UpdateFirmwareOperation extends AbstractMiBandOperation {
 
                 if ((i > 0) && (i % 50 == 0)) {
                     builder.write(getCharacteristic(MiBandService.UUID_CHARACTERISTIC_CONTROL_POINT), new byte[]{MiBandService.COMMAND_SYNC});
-                    builder.add(new SetProgressAction("Firmware update in progress", true, (int) (((float) firmwareProgress) / len * 100), getContext()));
+                    builder.add(new SetProgressAction(getContext().getString(R.string.updatefirmwareoperation_update_in_progress), true, (int) (((float) firmwareProgress) / len * 100), getContext()));
                 }
 
                 LOG.info("Firmware update progress:" + firmwareProgress + " total len:" + len + " progress:" + (int) (((float) firmwareProgress) / len * 100));
@@ -298,14 +299,14 @@ public class UpdateFirmwareOperation extends AbstractMiBandOperation {
             if (firmwareProgress >= len) {
                 builder.write(getCharacteristic(MiBandService.UUID_CHARACTERISTIC_CONTROL_POINT), new byte[]{MiBandService.COMMAND_SYNC});
             } else {
-                GB.updateInstallNotification("Firmware write failed", false, 0, getContext());
+                GB.updateInstallNotification(getContext().getString(R.string.updatefirmwareoperation_write_failed), false, 0, getContext());
             }
 
             builder.queue(getQueue());
 
         } catch (IOException ex) {
             LOG.error("Unable to send fw to MI", ex);
-            GB.updateInstallNotification("Firmware write failed", false, 0, getContext());
+            GB.updateInstallNotification(getContext().getString(R.string.updatefirmwareoperation_write_failed), false, 0, getContext());
             return false;
         }
         return true;
@@ -329,6 +330,7 @@ public class UpdateFirmwareOperation extends AbstractMiBandOperation {
                 TransactionBuilder builder = performInitialized("send firmware info");
                 builder.add(new SetDeviceBusyAction(getDevice(), getContext().getString(R.string.updating_firmware), getContext()));
                 builder.write(getCharacteristic(MiBandService.UUID_CHARACTERISTIC_CONTROL_POINT), getFirmwareInfo());
+                builder.add(new FirmwareInfoSucceededAction());
                 builder.queue(getQueue());
                 return true;
             } catch (IOException e) {
@@ -442,4 +444,12 @@ public class UpdateFirmwareOperation extends AbstractMiBandOperation {
             return false;
         }
     }
+
+    private class FirmwareInfoSucceededAction extends PlainAction {
+        @Override
+        public boolean run(BluetoothGatt gatt) {
+            firmwareInfoSent = true;
+            return true;
+        }
+    }
 }

app/src/main/res/values/strings.xml

@@ -235,5 +235,6 @@
     <string name="device_fw">FW: %1$s</string>
     <string name="error_creating_directory_for_logfiles">Error creating directory for log files: %1$s</string>
     <string name="DEVINFO_HR_VER">"HR: "</string>
+    <string name="updatefirmwareoperation_update_in_progress">Firmware update in progress</string>
 
 </resources>

app/src/test/java/nodomain/freeyourgadget/gadgetbridge/service/devices/miband/FirmwareTest.java

@@ -36,6 +36,8 @@ public class FirmwareTest {
         Assert.assertNotNull(wholeFw);
 
         AbstractMiFirmwareInfo info = getFirmwareInfo(wholeFw, SINGLE);
+        info.checkValid();
+
         int calculatedVersion = info.getFirmwareVersion();
         String version = MiBandFWHelper.formatFirmwareVersion(calculatedVersion);
         Assert.assertTrue(version.startsWith("1."));
@@ -49,6 +51,8 @@ public class FirmwareTest {
         Assert.assertNotNull(wholeFw);
 
         AbstractMiFirmwareInfo info = getFirmwareInfo(wholeFw, SINGLE);
+        info.checkValid();
+
         int calculatedVersion = info.getFirmwareVersion();
         String version = MiBandFWHelper.formatFirmwareVersion(calculatedVersion);
         Assert.assertTrue(version.startsWith("5."));
@@ -62,20 +66,9 @@ public class FirmwareTest {
         Assert.assertNotNull(wholeFw);
 
         AbstractMiFirmwareInfo info = getFirmwareInfo(wholeFw, DOUBLE);
+        info.checkValid();
 
         // Mi Band version
-        int calculatedLengthFw1 = info.getFirst().getFirmwareLength();
-        int calculatedOffsetFw1 = info.getFirst().getFirmwareOffset();
-        int endIndexFw1 = calculatedOffsetFw1 + calculatedLengthFw1;
-
-        int calculatedLengthFw2 = info.getSecond().getFirmwareLength();
-        int calculatedOffsetFw2 = info.getSecond().getFirmwareOffset();
-        int endIndexFw2 = calculatedOffsetFw2 + calculatedLengthFw2;
-
-        Assert.assertTrue(endIndexFw1 <= wholeFw.length - calculatedLengthFw2);
-        Assert.assertTrue(endIndexFw2 <= wholeFw.length);
-
-        Assert.assertTrue(endIndexFw1 <= calculatedOffsetFw2);
         int calculatedVersionFw1 = info.getFirst().getFirmwareVersion();
 //        Assert.assertEquals("Unexpected firmware 1 version: " + calculatedVersionFw1, MI1S_FW1_VERSION, calculatedVersionFw1);
         String version1 = MiBandFWHelper.formatFirmwareVersion(calculatedVersionFw1);
@@ -93,12 +86,48 @@ public class FirmwareTest {
         } catch (UnsupportedOperationException expected) {
         }
 
-        Assert.assertNotEquals(info.getFirst().getFirmwareOffset(), info.getSecond().getFirmwareOffset());
         Assert.assertFalse(Arrays.equals(info.getFirst().getFirmwareBytes(), info.getSecond().getFirmwareBytes()));
     }
 
+    @Test
+    public void testDoubleFirmwareMi1A() throws Exception {
+        byte[] wholeFw = getFirmwareMi1A();
+        Assert.assertNotNull(wholeFw);
+
+        AbstractMiFirmwareInfo info = TestMi1AFirmwareInfo.getInstance(wholeFw);
+        Assert.assertNotNull(info);
+        info.checkValid();
+
+        // Mi Band version
+        int calculatedVersionFw1 = info.getFirst().getFirmwareVersion();
+//        Assert.assertEquals("Unexpected firmware 1 version: " + calculatedVersionFw1, MI1S_FW1_VERSION, calculatedVersionFw1);
+        String version1 = MiBandFWHelper.formatFirmwareVersion(calculatedVersionFw1);
+        Assert.assertTrue(version1.startsWith("5."));
+
+        // Same Mi Band version
+        int calculatedVersionFw2 = info.getSecond().getFirmwareVersion();
+//        Assert.assertEquals("Unexpected firmware 2 version: " + calculatedVersionFw2, MI1S_FW2_VERSION, calculatedVersionFw2);
+        String version2 = MiBandFWHelper.formatFirmwareVersion(calculatedVersionFw2);
+        Assert.assertTrue(version2.startsWith("5."));
+
+        try {
+            info.getFirmwareVersion();
+            Assert.fail("should not get fw version from AbstractMi1SFirmwareInfo");
+        } catch (UnsupportedOperationException expected) {
+        }
+
+        // these are actually the same with this test info!
+        Assert.assertEquals(info.getFirst().getFirmwareOffset(), info.getSecond().getFirmwareOffset());
+        Assert.assertTrue(Arrays.equals(info.getFirst().getFirmwareBytes(), info.getSecond().getFirmwareBytes()));
+    }
+
     private AbstractMiFirmwareInfo getFirmwareInfo(byte[] wholeFw, int numFirmwares) {
         AbstractMiFirmwareInfo info = AbstractMiFirmwareInfo.determineFirmwareInfoFor(wholeFw);
+        assertFirmwareInfo(info, wholeFw, numFirmwares);
+        return info;
+    }
+
+    private AbstractMiFirmwareInfo assertFirmwareInfo(AbstractMiFirmwareInfo info, byte[] wholeFw, int numFirmwares) {
         switch (numFirmwares) {
             case SINGLE: {
                 Assert.assertTrue("should be single miband firmware", info.isSingleMiBandFirmware());