From bad9bbe54bd91ccc572b27d9c51e8d091b0e811c Mon Sep 17 00:00:00 2001 From: Elias Bakken Date: Fri, 18 Sep 2026 19:22:57 +0200 Subject: [PATCH] Offer released images only, newest first, and keep the panel moving (#170, #171, #172) Three things from one pass over the picker and the screen. The picker showed whatever order GitHub's payload happened to arrive in, which put the newest image somewhere down the list rather than at the top (#171), and it offered every release ever made - back to v0.1.0, which Reflash has never been tested against and which is not recommended (#172). Both are now decided in one pure function: releases at or above 1.0.2, drafts never, prereleases only when asked for, sorted newest first, each image listed once. Its own module because it is pure - payload in, list out - and that is the part worth testing; the de-duplication also settles the double fetch from created() and checkInternet() without having to chase which of them wins. "Show pre-releases" is a switch in the options panel rather than a decision taken here: released images are what someone flashing a printer wants by default, and the RCs are what this bench spends its days installing. The page keeps the releases payload it already fetched and the list is computed from it, so flicking the switch re-filters what is in hand instead of asking GitHub again - a toggle should not spend a round trip, or a share of an unauthenticated rate limit, to answer a question about a list already on screen. The screen (#170): the redraw lived inside refreshProgress, which runs from the HTTP poll and the USB STATUS command - both client-driven. A flash started from Recore-CI, from the control protocol, or simply with the browser closed ran to completion behind a frozen panel. The watchdog that already ticks every 500ms now samples while an operation is running, so the panel follows the board's own clock and not the browser's. Idle costs nothing: an idle board has nothing new to say, and its screen is drawn by the events that change it. The progress file the scripts write became a var so the new tests can point it at their sandbox; the helper is explicit that a test must not write real paths a running server might have open. Co-Authored-By: Claude Opus 5 --- client/src/App.vue | 48 +++++----- client/src/components/TheOptions.vue | 11 +++ client/src/rebuildImages.js | 56 +++++++++++ client/tests/unit/rebuildImages.spec.js | 118 ++++++++++++++++++++++++ reflash/server.go | 48 ++++++++-- reflash/server_test.go | 42 +++++++++ 6 files changed, 293 insertions(+), 30 deletions(-) create mode 100644 client/src/rebuildImages.js create mode 100644 client/tests/unit/rebuildImages.spec.js diff --git a/client/src/App.vue b/client/src/App.vue index 8e73bf5..a73fa15 100644 --- a/client/src/App.vue +++ b/client/src/App.vue @@ -246,6 +246,7 @@ import TheWifiSetup from "./components/TheWifiSetup"; import WaveUI from "wave-ui"; import { mapGetters, mapActions } from "vuex"; import axios from "axios"; +import { selectRebuildImages } from "./rebuildImages"; export default { name: "App", @@ -288,7 +289,10 @@ export default { selectedUploadImage: [], selectedLocalImage: undefined, githubImages: [], - rebuildImages: [], + // The releases payload as GitHub sent it. Kept raw so the picker can be + // filtered here rather than re-fetched: flicking "show pre-releases" is a + // question about a list we already have. + githubReleases: [], localImages: [], uploadError: false, openInfo: false, @@ -333,6 +337,12 @@ export default { ); }, ...mapGetters(["options", "progress", "flash"]), + // Derived from the cached payload, so the pre-release switch re-filters a + // list that is already here instead of asking GitHub again - and so the + // order does not depend on what the API happened to return (#171, #172). + rebuildImages() { + return selectRebuildImages(this.githubReleases, this.options.showPrereleases); + }, // Rebuild downloads from GitHub via the board itself (flash-from-url) - // without internet that can't work, so hide it and leave only the // local-file paths (upload, magic, install) - #74. @@ -1024,20 +1034,6 @@ export default { } } }, - populateRebuildImages(releases) { - for (let release of releases) { - for (let asset of release.assets) { - if (asset.name.includes("rebuild")) { - this.rebuildImages.push({ - name: asset.name, - id: asset.id, - url: asset.browser_download_url, - size: asset.size, - }); - } - } - } - }, // Static for as long as the page is open, and every field costs a partition // mount on the board - so this is fetched once and never on a state change. async getInfo() { @@ -1084,14 +1080,22 @@ export default { setTimeout(this.getStatus, 1000); } }, + // Only fetches. What the picker shows - which releases, in what order - is + // decided by the rebuildImages computed, so this can be called twice (see + // checkInternet()) without duplicating anything, and the pre-release switch + // re-filters what is already here rather than asking GitHub again. async getGithubImages() { - // Reset first - this can now be called again (see checkInternet()) - // after already having been called once, and populateRebuildImages - // appends rather than replaces. - this.rebuildImages = []; - fetch("https://api.github.com/repos/intelligent-agent/Rebuild/releases") - .then((response) => response.json()) - .then((data) => this.populateRebuildImages(data)); + try { + const response = await fetch( + "https://api.github.com/repos/intelligent-agent/Rebuild/releases" + ); + this.githubReleases = await response.json(); + } catch (err) { + // No internet, or GitHub rate-limiting this board's address. The empty + // picker already says there is nothing to download, and the methods + // list hides Download without internet (#74). + this.githubReleases = []; + } }, async checkInternet() { const response = await axios.get(`/api/has_internet`); diff --git a/client/src/components/TheOptions.vue b/client/src/components/TheOptions.vue index 3d3168f..ad1eec0 100644 --- a/client/src/components/TheOptions.vue +++ b/client/src/components/TheOptions.vue @@ -31,6 +31,17 @@ label="Magicmode" > + + +

Screen rotation

[1, 1, 0]. The prerelease suffix is deliberately dropped: a +// release's *version* is what the floor is about, and whether it is a +// prerelease is already a field of its own on the release. +function versionOf(tag) { + const m = /^v?(\d+)\.(\d+)\.(\d+)/.exec(String(tag || '')); + return m ? [Number(m[1]), Number(m[2]), Number(m[3])] : null; +} + +function compare(a, b) { + for (let i = 0; i < 3; i++) { + if (a[i] !== b[i]) return a[i] - b[i]; + } + return 0; +} + +// The list the picker shows, newest first. +// +// Sorted here rather than taken as it arrives: the order used to be whatever +// the API returned, which put the newest image somewhere down the list (#171). +// De-duplicated for the same reason - the picker is fed from two places that +// can both land, and a list with an image in it twice is a list nobody trusts. +export function selectRebuildImages(releases, showPrereleases) { + if (!Array.isArray(releases)) return []; // an error body is not a list + + const wanted = releases + .filter((r) => r && !r.draft) + .filter((r) => showPrereleases || !r.prerelease) + .map((r) => ({ release: r, version: versionOf(r.tag_name) })) + .filter(({ version }) => version && compare(version, OLDEST_OFFERED) >= 0) + .sort((a, b) => compare(b.version, a.version)); + + const seen = new Set(); + const images = []; + for (const { release } of wanted) { + const assets = (release.assets || []) + .filter((a) => a && typeof a.name === 'string' && a.name.startsWith('rebuild-')) + // Within one release the order is the variants', and GitHub's asset order + // is upload order - which is not something anyone chose. + .sort((a, b) => a.name.localeCompare(b.name)); + for (const a of assets) { + if (seen.has(a.name)) continue; + seen.add(a.name); + images.push({ name: a.name, id: a.id, url: a.browser_download_url, size: a.size }); + } + } + return images; +} diff --git a/client/tests/unit/rebuildImages.spec.js b/client/tests/unit/rebuildImages.spec.js new file mode 100644 index 0000000..c2f7d18 --- /dev/null +++ b/client/tests/unit/rebuildImages.spec.js @@ -0,0 +1,118 @@ +import { describe, it, expect } from 'vitest'; +import { selectRebuildImages } from '@/rebuildImages'; + +// Shaped like the GitHub releases payload, with only the fields the picker uses. +function release(tag, { prerelease = false, draft = false, variants = ['barebone', 'fluidd'] } = {}) { + return { + tag_name: tag, + prerelease, + draft, + assets: variants.map((v, i) => ({ + name: `rebuild-${v}-${tag}.img.xz`, + id: `${tag}-${i}`, + browser_download_url: `http://example/${v}-${tag}`, + size: 1000 + i, + })), + }; +} + +const names = (list) => list.map((i) => i.name); + +describe('the Rebuild image picker', () => { + // The order used to be whatever the API handed back. It is the newest image + // people want, so it belongs at the top whatever GitHub does (#171). + it('puts the newest release first, whatever order the payload arrives in', () => { + const out = selectRebuildImages( + [release('v1.0.2'), release('v1.1.0'), release('v1.0.3')], + false + ); + expect(names(out)[0]).toContain('v1.1.0'); + expect(names(out)).toEqual([ + 'rebuild-barebone-v1.1.0.img.xz', 'rebuild-fluidd-v1.1.0.img.xz', + 'rebuild-barebone-v1.0.3.img.xz', 'rebuild-fluidd-v1.0.3.img.xz', + 'rebuild-barebone-v1.0.2.img.xz', 'rebuild-fluidd-v1.0.2.img.xz', + ]); + }); + + // Reflash has not been tested against them and they are not recommended + // (#172). 1.0.2 is the previous release and the floor. + it('drops releases older than 1.0.2', () => { + const out = selectRebuildImages( + [release('v1.0.2'), release('v1.0.1'), release('v1.0.0'), release('v0.1.0')], + false + ); + expect(names(out).every((n) => n.includes('v1.0.2'))).toBe(true); + }); + + it('hides prereleases unless they are asked for', () => { + const payload = [release('v1.1.0-RC8', { prerelease: true }), release('v1.0.2')]; + + expect(names(selectRebuildImages(payload, false)).some((n) => n.includes('RC8'))).toBe(false); + expect(names(selectRebuildImages(payload, true))[0]).toContain('RC8'); + }); + + // A prerelease of a version below the floor is still below the floor. + it('applies the floor to prereleases too', () => { + const out = selectRebuildImages([release('v1.0.1-RC1', { prerelease: true })], true); + expect(out).toEqual([]); + }); + + it('never shows drafts', () => { + const out = selectRebuildImages([release('v1.2.0', { draft: true }), release('v1.0.2')], true); + expect(names(out).some((n) => n.includes('v1.2.0'))).toBe(false); + }); + + // The picker is fed by two calls that can both land (created() and + // checkInternet()), and a duplicated list is a list nobody trusts. + it('lists each image once even if the payload is repeated', () => { + const payload = [release('v1.0.2')]; + const out = selectRebuildImages([...payload, ...payload], false); + expect(names(out)).toEqual([ + 'rebuild-barebone-v1.0.2.img.xz', + 'rebuild-fluidd-v1.0.2.img.xz', + ]); + }); + + // Only Rebuild images belong in a Rebuild picker; a release can carry other + // assets (checksums, a Reflash image someone attached by hand). + it('ignores assets that are not Rebuild images', () => { + const rel = release('v1.0.2'); + rel.assets.push({ name: 'SHA256SUMS', id: 'x', browser_download_url: 'u', size: 1 }); + rel.assets.push({ name: 'reflash-v1.0.2.img.xz', id: 'y', browser_download_url: 'u', size: 1 }); + expect(names(selectRebuildImages([rel], false))).toEqual([ + 'rebuild-barebone-v1.0.2.img.xz', + 'rebuild-fluidd-v1.0.2.img.xz', + ]); + }); + + it('survives a payload that is not a list', () => { + expect(selectRebuildImages(undefined, false)).toEqual([]); + expect(selectRebuildImages({ message: 'API rate limit exceeded' }, false)).toEqual([]); + }); + + it('keeps the fields the picker and the download need', () => { + const [first] = selectRebuildImages([release('v1.0.2')], false); + expect(first).toEqual({ + name: 'rebuild-barebone-v1.0.2.img.xz', + id: 'v1.0.2-0', + url: 'http://example/barebone-v1.0.2', + size: 1000, + }); + }); +}); + +// The switch is a question about a list the page already has. Re-fetching to +// answer it would put a GitHub round trip - and its rate limit - behind a +// toggle. +describe('the pre-release switch re-filters what is already loaded', () => { + it('needs only the cached payload to change the list', () => { + const payload = [release('v1.1.0-RC8', { prerelease: true }), release('v1.0.2')]; + + const hidden = selectRebuildImages(payload, false); + const shown = selectRebuildImages(payload, true); + + expect(hidden).toHaveLength(2); + expect(shown).toHaveLength(4); + expect(names(shown)[0]).toContain('RC8'); + }); +}); diff --git a/reflash/server.go b/reflash/server.go index a6929c2..6c20fd0 100644 --- a/reflash/server.go +++ b/reflash/server.go @@ -149,13 +149,18 @@ type AccessPoint struct { } type Options struct { - Darkmode bool `json:"darkmode"` - RebootWhenDone bool `json:"rebootWhenDone"` - EnableSsh bool `json:"enableSsh"` - Magicmode bool `json:"magicmode"` - ScreenRotation int `json:"screenRotation"` - WifiSSID string `json:"SSID"` - WifiPSK string `json:"PSK"` + Darkmode bool `json:"darkmode"` + RebootWhenDone bool `json:"rebootWhenDone"` + EnableSsh bool `json:"enableSsh"` + Magicmode bool `json:"magicmode"` + // Off by default, so the picker offers only released images. Reflash has + // not been tested against prereleases any more than against the old + // versions the picker now hides, and an RC is not what someone flashing a + // printer wants by accident (#172). + ShowPrereleases bool `json:"showPrereleases"` + ScreenRotation int `json:"screenRotation"` + WifiSSID string `json:"SSID"` + WifiPSK string `json:"PSK"` } type Download struct { @@ -237,6 +242,12 @@ var binDir string var images_folder string var options_file string var log_file string + +// Where the flashing scripts report their progress. A var rather than the +// literal it used to be so a test can point it at its sandbox: the helper +// refuses to write real paths, because a test must not scribble on what a +// server running on the same machine has open. +var flash_progress_file = "/tmp/recore-flash-progress" var http_port string var reflashVersion string @@ -1764,6 +1775,20 @@ func resetTransfer() { bytesAtLastLog = 0 } +// The watchdog's share of the redraw: sample and repaint while an operation is +// running, and cost nothing when one is not. +// +// Only while busy, because refreshProgress is not free - it reads the progress +// file, recomputes the bandwidth and redraws the panel - and an idle board has +// nothing new to say. The idle screen is repainted by the events that change +// it (drive in, drive out, state transitions), which is where it always was. +func refreshProgressWhenBusy() { + switch state.State { + case DOWNLOADING, UPLOADING, INSTALLING, BACKUPING, MAGIC, UPLOADING_MAGIC: + refreshProgress() + } +} + // refreshProgress reads the active progress source (the flash-progress file // while installing/backing up/magicking, or the downloading file's size), // recomputes state.Progress + state.Bandwidth, and redraws the embedded @@ -1771,7 +1796,7 @@ func resetTransfer() { // dispatcher so both paths keep the on-board display alive. func refreshProgress() { if state.State == INSTALLING || state.State == BACKUPING || state.State == MAGIC { - bytes := lastLine("/tmp/recore-flash-progress") + bytes := lastLine(flash_progress_file) i, err := strconv.Atoi(bytes) if err != nil { i = 0 @@ -2744,6 +2769,13 @@ var startWatchdog = func() { // Give up on an upload whose client has gone away, rather than // sitting in UPLOADING with the drive mounted rw forever (#118). checkUploadLiveness() + // Keep the embedded screen moving while something is running, even + // with nothing watching. The redraw lived only inside + // refreshProgress, which runs from the HTTP poll and the USB STATUS + // command - both client-driven - so a flash started from the CI, or + // from the control protocol, or simply with the browser closed, ran + // to completion behind a frozen panel (#170). + refreshProgressWhenBusy() } }() } diff --git a/reflash/server_test.go b/reflash/server_test.go index 6d6e65a..3109a72 100644 --- a/reflash/server_test.go +++ b/reflash/server_test.go @@ -48,6 +48,8 @@ func setupTest(t *testing.T) string { // Never the real /tmp/mypipe: a test must not write into whatever a // running server on the same machine has open. magic_pipe = filepath.Join(dir, "mypipe") + // Same reasoning for where the flashing scripts report progress. + flash_progress_file = filepath.Join(dir, "recore-flash-progress") // slowInit starts the real watchdog, which would outlive the test and act // on the state of whichever test runs next. startWatchdog = func() {} @@ -1910,3 +1912,43 @@ func TestCancelledUploadKeepsTheImageAlreadyOnTheDrive(t *testing.T) { t.Error("the partial file was left behind") } } + +// The embedded screen used to move only while something was polling the +// server: the redraw lived inside refreshProgress, which runs from the HTTP +// poll and the USB STATUS command. A flash started from the CI, or with the +// browser closed, therefore ran to completion behind a frozen panel (#170). +// The watchdog now samples on the board's own clock while an operation runs. +func TestProgressIsSampledWithoutAClientWatching(t *testing.T) { + setupTest(t) + state = &State{State: INSTALLING, BytesTotal: 1000} + if err := os.WriteFile(flash_progress_file, []byte("400\n"), 0o644); err != nil { + t.Fatal(err) + } + + refreshProgressWhenBusy() + + if state.BytesNow != 400 { + t.Errorf("bytes_now = %d, want 400 - the watchdog did not sample the progress file", + state.BytesNow) + } + if state.Progress != 40 { + t.Errorf("progress = %.1f%%, want 40%%", state.Progress) + } +} + +// An idle board has nothing new to say, and refreshProgress is not free - it +// reads a file, recomputes the bandwidth and repaints. The idle screen is +// drawn by the events that change it instead. +func TestAnIdleBoardIsNotResampled(t *testing.T) { + setupTest(t) + state = &State{State: IDLE, BytesTotal: 1000, BytesNow: 7} + if err := os.WriteFile(flash_progress_file, []byte("400\n"), 0o644); err != nil { + t.Fatal(err) + } + + refreshProgressWhenBusy() + + if state.BytesNow != 7 { + t.Errorf("bytes_now = %d, want it untouched at 7", state.BytesNow) + } +}