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) + } +}