Fix the nine issues found testing Reflash on an A8 (#149-#157)

bin/prod/save-settings

@@ -19,7 +19,9 @@ update_settings(){
     umount -q /mnt/emmc/
     mount "${EMMC}p2" /mnt/emmc
 
-    echo -e "$CONTENT" > "$SETTINGS_FILE"
+    # printf, not echo -e: the content arrives with real newlines, and -e
+    # also turned any backslash in a passphrase into an escape (#157).
+    printf '%s\n' "$CONTENT" > "$SETTINGS_FILE"
     chmod 600 "$SETTINGS_FILE"
     umount /mnt/emmc
 }

bin/prod/wifi-connect

@@ -18,11 +18,41 @@ info() {
 # A failed connection here (bad password, AP out of range, DHCP timeout)
 # would otherwise leave the board in station mode with no working
 # connection and no hotspot - unreachable until physical intervention (#90).
-restore_hotspot() {
-    info "Connection failed - restoring hotspot ($AP_PROFILE) so the board stays reachable."
+# Whether the adapter is actually serving the AP now.
+hotspot_up() {
+    [ "$(iwctl device list 2>/dev/null \
+        | sed $'s/\x1b\[[0-9;]*m//g' \
+        | awk -v i="$INTERFACE" '$1 == i {print $NF}')" = "ap" ]
+}
+
+start_hotspot() {
     iwctl ap "$INTERFACE" stop 2>/dev/null
     iwctl device "$INTERFACE" set-property Mode ap
     iwctl ap "$INTERFACE" start-profile "$AP_PROFILE" 2>/dev/null
+    sleep 1
+    hotspot_up
+}
+
+restore_hotspot() {
+    restore_profile
+    info "Connection failed - restoring hotspot ($AP_PROFILE) so the board stays reachable."
+    if start_hotspot; then
+        return 0
+    fi
+    # Checked rather than assumed, because it has been seen not to happen:
+    # iwd 3.8 segfaulted during this on an A8 (status=11/SEGV, while
+    # `iwctl ap start-profile` ran), systemd restarted it, and it came back in
+    # station mode - so the board had no hotspot at all, which is the state
+    # this function exists to prevent (#90). One retry against the restarted
+    # daemon is enough to get it back.
+    info "The hotspot did not come up - retrying once."
+    sleep 2
+    if start_hotspot; then
+        info "Hotspot restored on the second attempt."
+        return 0
+    fi
+    info "WARNING: could not start the $AP_PROFILE hotspot. The board is reachable over Ethernet only."
+    return 1
 }
 
 if [ -z "$SSID" ] || [ -z "$PASS" ]; then
@@ -50,9 +80,36 @@ if [ "$CURRENT_MODE" == "ap" ]; then
 fi
 
 # 2. Create the iwd profile
+#
+# Kept aside first, because a failed attempt has to leave iwd as it found it.
+# The profile carries AutoConnect=true, so a wrong passphrase left behind was
+# retried by iwd on its own and on every boot - and a mistyped retry would
+# overwrite a profile that worked (#150).
+PROFILE="$IWD_DIR/$SSID.psk"
+PREV_PROFILE=""
+if [ -f "$PROFILE" ]; then
+    PREV_PROFILE=$(mktemp)
+    cp -p "$PROFILE" "$PREV_PROFILE"
+fi
+
+# Put back what was there before this attempt, or nothing if there was nothing.
+# forget first: iwd holds known networks in memory as well as on disk, and
+# deleting the file alone would leave it retrying the passphrase that failed.
+restore_profile() {
+    iwctl known-networks "$SSID" forget 2>/dev/null
+    rm -f "$PROFILE"
+    if [ -n "$PREV_PROFILE" ]; then
+        cp -p "$PREV_PROFILE" "$PROFILE"
+        rm -f "$PREV_PROFILE"
+        info "Restored the previous profile for $SSID."
+    else
+        info "Removed the new profile for $SSID."
+    fi
+}
+
 info "Provisioning iwd profile for $SSID..."
 mkdir -p "$IWD_DIR"
-cat <<IWD_EOF > "$IWD_DIR/$SSID.psk"
+cat <<IWD_EOF > "$PROFILE"
 [Settings]
 AutoConnect=true
 
@@ -117,7 +174,28 @@ else
     info "$SSID did not appear in a scan within ${SCAN_TIMEOUT}s - trying anyway. If the connect also fails, check the name, the signal, and whether this adapter can use that band."
 fi
 
+# The network iwd says it is on right now, if any.
+connected_network() {
+    iwctl station "$INTERFACE" show 2>/dev/null \
+        | sed $'s/\x1b\[[0-9;]*m//g' \
+        | sed -n 's/^[[:space:]]*Connected network[[:space:]]\{2,\}//p' \
+        | sed 's/[[:space:]]*$//'
+}
+
 # 4. Trigger the connection
+#
+# Drop the current association first. Without this, asking to connect to the
+# network the board is already on did nothing at all: iwd stays associated with
+# the session it already has, the address never goes away, and the wait below
+# saw it immediately and reported success - for a passphrase that had never been
+# tested. A wrong one then looked like it worked and was saved as if it did.
+ALREADY=$(connected_network)
+if [ -n "$ALREADY" ]; then
+    info "Disconnecting from $ALREADY first, so the passphrase is actually tested."
+    iwctl station "$INTERFACE" disconnect 2>/dev/null
+    sleep 2
+fi
+
 info "Requesting iwd to connect..."
 # Still retried, but now only for a genuinely transient refusal - the slow-scan
 # case it used to paper over is handled above.
@@ -183,6 +261,10 @@ install_source_route() {
 }
 
 # 5. Wait and verify IP address
+#
+# Trustworthy because of the disconnect above: iwd drops the lease with the
+# association, so an address here belongs to the network just asked for rather
+# than to the session the board already had.
 info "Waiting for DHCP..."
 MAX_RETRIES=30
 COUNT=0
@@ -191,6 +273,7 @@ while [ $COUNT -lt $MAX_RETRIES ]; do
     if [ -n "$IP" ]; then
         info "Success! Connected with IP: $IP"
         install_source_route "${IP%%/*}"
+        [ -n "$PREV_PROFILE" ] && rm -f "$PREV_PROFILE"
         exit 0
     fi
     sleep 1

bin/prod/wifi-scan

@@ -39,10 +39,14 @@ sleep 3
 # 4. Structured Output
 echo "---SCAN_RESULTS_START---"
 # This pipeline:
-# 1. sed: Nukes ANSI escape codes (the \x1B[...m stuff)
-# 2. sed: Removes the > marker
-# 3. awk: Parses the columns safely
+# 1. sed: Drops the grey stars. iwctl always prints four, and draws the unlit
+#    ones in grey (`***\e[1;90m*\e[0m` is three bars), so stripping the colour
+#    first turned every network into "****" whatever its signal (#154).
+# 2. sed: Nukes the remaining ANSI escape codes (the \x1B[...m stuff)
+# 3. sed: Removes the > marker
+# 4. awk: Parses the columns safely
 iwctl station "$INTERFACE" get-networks | \
+    sed $'s/\x1b\[1;90m\*\+//g' | \
     sed $'s/\x1b\[[0-9;]*m//g' | \
     sed 's/>//g' | \
     awk '

client/src/App.vue

@@ -177,6 +177,15 @@
             ref="integritychecker"
             v-if="!options.magicmode"
             @integrity="imageIntegrity = $event" />
+          <w-button
+            text
+            class="ml1"
+            v-if="isDeleteButtonVisible()"
+            :title="'Delete ' + selectedLocalImage + ' from the USB drive'"
+            @click="onDeleteImageClick()"
+          >
+            <span>{{ confirmDelete ? "Delete?" : "Delete" }}</span>
+          </w-button>
         </w-flex>
         <div class="xs1 align-self-center">
           <w-button
@@ -255,6 +264,9 @@ export default {
     ownsUpload: false,
     // null = unknown or still checking, true = passed, false = failed.
     imageIntegrity: null,
+    // Delete takes two clicks, the second within a few seconds.
+    confirmDelete: false,
+    confirmDeleteTimer: null,
     state: "IDLE",
     previousState: "IDLE",
     installFinished: false,
@@ -379,11 +391,16 @@ export default {
     // Disabled rather than hidden: a button that vanishes reads as a bug, where
     // a greyed-out one next to a red X explains itself.
     isInstallButtonDisabled() {
+      // During its own operation this button means Cancel, which must stay
+      // reachable regardless of what the image turned out to be.
+      if (this.state == "INSTALLING" || this.state == "BACKUPING") return false;
+      // During anything else it is neither. It read "Install" and stayed
+      // enabled through a download or an upload, and clicking it sent
+      // cancel_installation - or cancel_backup - at a transfer it had nothing
+      // to do with (#156).
+      if (this.state != "IDLE") return true;
       // Backups write no image, so there is nothing to verify.
       if (this.flash.selectedMethod == 1) return false;
-      // Once something is running this button means Cancel, which must stay
-      // reachable regardless of what the image turned out to be.
-      if (this.state != "IDLE") return false;
       // null covers both "still checking" and "the check did not answer".
       // Neither is a pass, and enabling on either is how a truncated image
       // gets flashed during the second the spinner is up.
@@ -863,17 +880,53 @@ export default {
       if (this.flash.selectedMethod == 0) {
         if (this.state == "IDLE") {
           this.installSelected();
-        } else {
+        } else if (this.state == "INSTALLING") {
           this.apiCall("cancel_installation");
         }
       } else {
         if (this.state == "IDLE") {
           this.backupSelected();
-        } else {
+        } else if (this.state == "BACKUPING") {
           this.apiCall("cancel_backup");
         }
       }
     },
+    isDeleteButtonVisible() {
+      return (
+        !this.options.magicmode &&
+        this.flash.selectedMethod == 0 &&
+        !!this.selectedLocalImage &&
+        this.state == "IDLE"
+      );
+    },
+    // A truncated image - an abandoned upload, an old cancelled backup - could
+    // only ever fail its integrity check, and there was no way to remove it
+    // (#153). Two clicks rather than confirm(): a native dialog blocks the
+    // page, progress polling included.
+    async onDeleteImageClick() {
+      clearTimeout(this.confirmDeleteTimer);
+      if (!this.confirmDelete) {
+        this.confirmDelete = true;
+        this.confirmDeleteTimer = setTimeout(() => (this.confirmDelete = false), 4000);
+        return;
+      }
+      this.confirmDelete = false;
+      const name = this.selectedLocalImage;
+      try {
+        const res = await axios.put(`/api/delete_image`, { filename: name });
+        if (res.data.status == "ERROR") {
+          this.$waveui.notify(res.data.error, "error", 0);
+        }
+      } catch (err) {
+        this.$waveui.notify(
+          "Could not delete " + name + ": " + (err.response?.data || err),
+          "error",
+          0
+        );
+      }
+      this.selectedLocalImage = null;
+      await this.getStatus();
+    },
     async installSelected() {
       let self = this;
       await axios
@@ -1008,6 +1061,8 @@ export default {
     // A watcher runs when the selection actually changes, which is also what
     // keeps the Install button's state from flickering while polling redraws.
     selectedLocalImage() {
+      // A pending "Delete?" belongs to the image it was clicked for.
+      this.confirmDelete = false;
       this.onSelectedFileChanged();
     },
   },

client/src/components/TheWifiSetup.vue

@@ -292,17 +292,43 @@ export default {
         this.boardReachable = false;
       }
 
+      // The server's own verdict on the attempt. The radio state alone cannot
+      // give it: iwd names the network while it is still *connecting*, so a
+      // wrong passphrase looked exactly like success for the few seconds before
+      // it failed, and the watch had stopped by then (#149). null when it
+      // cannot be read, which is not a verdict either way.
+      let attempt = null;
+      if (wifi && wantMode === "station") {
+        try {
+          const res = await axios.get('/api/wifi_poll_connect', { timeout: REQUEST_TIMEOUT });
+          attempt = res.data || null;
+        } catch (err) {
+          attempt = null;
+        }
+      }
+
       if (wifi) {
         if (wifi.mode !== this.reconnectFromMode) {
           this.sawTransition = true;
         }
+        if (wantMode === "station" && attempt && !attempt.isConnecting && attempt.error) {
+          this.wifi = wifi;
+          this.stopReconnectWatch();
+          this.statusMessage =
+            `Could not join ${target}. The board is back on its own Recore hotspot.`;
+          this.$waveui.notify(this.statusMessage, "error", 0);
+          return;
+        }
         // Only "arrived" once the board reports the state we asked for. Right
         // after the request it is still on the old one, and treating that as
-        // success would report a connection that has not happened.
+        // success would report a connection that has not happened. For a
+        // network that also means an address and an attempt that has finished:
+        // named-but-still-associating is not joined.
         const arrived =
           wantMode === "ap"
             ? wifi.mode === "ap"
-            : wifi.mode === "station" && wifi.ssid === target;
+            : wifi.mode === "station" && wifi.ssid === target && !!wifi.ip &&
+              !(attempt && attempt.isConnecting);
         if (arrived) {
           this.wifi = wifi;
           this.isWifiPresent = !!wifi.present;

client/tests/unit/InstallGating.spec.js

@@ -47,6 +47,82 @@ describe('Install is refused for an image that failed its integrity check', () =
   it.each(['INSTALLING', 'BACKUPING'])('stays clickable as Cancel during %s', (state) => {
     expect(disabled({ state, imageIntegrity: false })).toBe(false);
   });
+
+  // Through a transfer it read "Install" and stayed enabled, and a click sent
+  // cancel_installation or cancel_backup at the transfer (#156).
+  it.each(['DOWNLOADING', 'UPLOADING', 'MAGIC', 'UPLOADING_MAGIC'])(
+    'is disabled during %s, for install and backup alike', (state) => {
+      expect(disabled({ state, imageIntegrity: true })).toBe(true);
+      expect(disabled({ state, flash: { selectedMethod: 1 } })).toBe(true);
+    });
+
+  it.each([
+    ['DOWNLOADING', 0], ['UPLOADING', 0], ['UPLOADING', 1], ['MAGIC', 1],
+  ])('sends no cancel from a %s (method %i)', (state, selectedMethod) => {
+    const apiCall = vi.fn();
+    App.methods.onInstallButtonClick.call({
+      flash: { selectedMethod }, state, apiCall,
+      installSelected: vi.fn(), backupSelected: vi.fn(),
+    });
+    expect(apiCall).not.toHaveBeenCalled();
+  });
+});
+
+describe('deleting an image (#153)', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    vi.useFakeTimers();
+  });
+
+  function stand(over = {}) {
+    return {
+      selectedLocalImage: 'truncated.img.xz',
+      confirmDelete: false,
+      confirmDeleteTimer: null,
+      getStatus: vi.fn(),
+      $waveui: { notify: vi.fn() },
+      ...over,
+    };
+  }
+
+  it('asks first, and deletes on the second click', async () => {
+    axios.put.mockResolvedValue({ data: { status: 'OK' } });
+    const self = stand();
+    await App.methods.onDeleteImageClick.call(self);
+    expect(axios.put).not.toHaveBeenCalled();
+    expect(self.confirmDelete).toBe(true);
+
+    await App.methods.onDeleteImageClick.call(self);
+    expect(axios.put).toHaveBeenCalledWith('/api/delete_image', { filename: 'truncated.img.xz' });
+    expect(self.selectedLocalImage).toBe(null);
+    expect(self.getStatus).toHaveBeenCalled();
+  });
+
+  it('forgets the first click after a few seconds', async () => {
+    const self = stand();
+    await App.methods.onDeleteImageClick.call(self);
+    vi.advanceTimersByTime(5000);
+    expect(self.confirmDelete).toBe(false);
+  });
+
+  it('says why when the server refuses', async () => {
+    axios.put.mockResolvedValue({ data: { status: 'ERROR', error: 'busy: UPLOADING' } });
+    const self = stand({ confirmDelete: true });
+    await App.methods.onDeleteImageClick.call(self);
+    expect(self.$waveui.notify).toHaveBeenCalledWith('busy: UPLOADING', 'error', 0);
+  });
+
+  it('is only offered for an image, while nothing is running', () => {
+    const visible = (over) => App.methods.isDeleteButtonVisible.call({
+      options: { magicmode: false }, flash: { selectedMethod: 0 },
+      selectedLocalImage: 'a.img.xz', state: 'IDLE', ...over,
+    });
+    expect(visible({})).toBe(true);
+    expect(visible({ state: 'UPLOADING' })).toBe(false);
+    expect(visible({ selectedLocalImage: null })).toBe(false);
+    expect(visible({ options: { magicmode: true } })).toBe(false);
+    expect(visible({ flash: { selectedMethod: 1 } })).toBe(false);
+  });
 });
 
 describe('IntegrityChecker reports its verdict', () => {