diff --git a/openpilot/starpilot/controllers/tests/test_native_wheel_callbacks.py b/openpilot/starpilot/controllers/tests/test_native_wheel_callbacks.py index da404fb5b2..46c076ffde 100644 --- a/openpilot/starpilot/controllers/tests/test_native_wheel_callbacks.py +++ b/openpilot/starpilot/controllers/tests/test_native_wheel_callbacks.py @@ -70,6 +70,36 @@ def test_actual_favorite_value_callback_reads_units_and_publishes_absolute_recei assert len(sender.events) == 1 +@pytest.mark.parametrize('metric', [False, True]) +def test_galaxy_speed_edit_reaches_native_callback_without_activation_or_stale_overwrite(native_session, metric): + from openpilot.starpilot.galaxy.favorites import FavoritesGateway + from openpilot.starpilot.galaxy.settings import AuthorityContext + _, session, ui, sender, _ = native_session + ui.params.put_bool('IsMetric', metric, block=True) + slots = default_slots() + slots[0].update(enabled=True, show_onroad=False, key=SET_SPEED, label='Cruise', value=30) + slots[1].update(enabled=True, show_onroad=True, key=BOOKMARK, label='Mark') + owner = session.favorites_owner + owner.save(slots, owner.snapshot().revision) + old_request = owner.snapshot().slots[0].request + gateway = FavoritesGateway(ui.params, NS(sample=lambda: AuthorityContext(False, None, None))) + try: + data = gateway.snapshot() + data['slots'][0]['value'] = 45 + saved = gateway.save({'revision': data['revision'], 'slots': data['slots']}, session_valid=lambda: True) + assert saved['slots'] == [{**slots[0], 'value': 45}, *slots[1:]] + assert gateway.snapshot()['slots'] == saved['slots'] + assert sender.events == [] + assert not owner.invoke(old_request).success + assert sender.events == [] + assert owner.invoke(owner.snapshot().slots[0].request).success + event = messaging.log_from_bytes(sender.events[-1][1]) + assert str(event.slcAction.kind) == 'cruiseSet' + assert event.slcAction.controllerCruise.targetSpeedMps == pytest.approx(45 / 3.6 if metric else 45 * .44704) + finally: + gateway.close() + + def test_card_receipt_invokes_real_native_bookmark_and_quick_select_callback(native_session, monkeypatch): runtime, session, ui, _, calls = native_session source, publisher = WheelPublisher(ui.params), Publisher() diff --git a/openpilot/starpilot/galaxy/tests/test_favorites.mjs b/openpilot/starpilot/galaxy/tests/test_favorites.mjs index 676bd18363..43c74cee98 100644 --- a/openpilot/starpilot/galaxy/tests/test_favorites.mjs +++ b/openpilot/starpilot/galaxy/tests/test_favorites.mjs @@ -64,6 +64,57 @@ legacy.feed.update(1, { key: "RainbowPath" }); body = JSON.parse(legacy.requests assert.equal(Object.hasOwn(body.slots[1], "value"), false) assert.equal(body.slots[1].label, "Rainbow Road") +const speedData = () => { + const data = snapshot() + data.options.push({ key: SET_SPEED, label: "Set Speed To", kind: "action", section: "Driving", available: false, reason: "Use on device" }) + data.slots[0] = { enabled: true, show_onroad: false, key: SET_SPEED, label: "Cruise", value: 30 } + data.slots[1] = { enabled: false, show_onroad: true, key: "OldUnsupportedSetting", label: "Keep me" } + data.slots[2] = { enabled: true, show_onroad: true, key: SET_SPEED, label: "Other speed", value: 55 } + return data +} +const speed = fixture(); speed.feed.start(); await speed.reply(0, speedData()) +const speedBefore = copy(speed.feed.data) +FavoritesPage.methods.speed.call({ feed: speed.feed }, 0, { target: { value: "45" } }) +assert.equal(speed.requests.length, 2, "The speed field must submit its changed value") +body = JSON.parse(speed.requests[1].options.body) +assert.deepEqual(body, { revision: "source-1", slots: [{ ...speedBefore.slots[0], value: 45 }, ...speedBefore.slots.slice(1)] }) +assert.equal(speed.requests[1].options.method, "POST") +assert.equal(speed.requests[1].options.credentials, "same-origin") +assert.deepEqual(speed.feed.data, speedBefore) // Wait for confirmation, including speed edits. +speed.feed.update(0, { value: 60 }); assert.equal(speed.requests.length, 2) // Only one write in flight. +const speedSaved = { ...speedBefore, slots: body.slots, revision: "speed-saved" } +await speed.reply(1, speedSaved) +assert.equal(speed.feed.data.slots[0].value, 45) +speed.feed.load(); await speed.reply(2, speedSaved) +assert.equal(speed.feed.data.slots[0].value, 45) // A subsequent read retains the saved target. + +for (const value of [undefined, null, "45", true, NaN, Infinity, -Infinity, 4, 146]) { + speed.feed.update(0, { value }) + assert.equal(speed.requests.length, 3, "Invalid speed patches must not send a request") +} +speed.feed.update(1, { value: 45 }); speed.feed.update(0, { value: 45, arbitrary: true }) +assert.equal(speed.requests.length, 3) // Values belong only to Set Speed; unknown fields stay blocked. +for (const value of [5, 145, 45.5]) { + const bounded = fixture(); bounded.feed.start(); await bounded.reply(0, speedData()) + bounded.feed.update(0, { value }) + assert.equal(JSON.parse(bounded.requests[1].options.body).slots[0].value, value) + await bounded.reply(1, { ...speedData(), slots: JSON.parse(bounded.requests[1].options.body).slots }) +} +const readonlySpeed = fixture(); readonlySpeed.feed.start(); await readonlySpeed.reply(0, { ...speedData(), editable: false }) +readonlySpeed.feed.update(0, { value: 45 }); assert.equal(readonlySpeed.requests.length, 1) + +const staleSpeed = fixture(); staleSpeed.feed.start(); await staleSpeed.reply(0, speedData()) +staleSpeed.feed.update(0, { value: 45 }); await staleSpeed.reply(1, {}, 409) +assert.equal(staleSpeed.feed.data.slots[0].value, 30) +assert.equal(staleSpeed.feed.needsReload, true) +staleSpeed.feed.update(0, { value: 60 }); assert.equal(staleSpeed.requests.length, 2) +const speedTimeout = fixture(); speedTimeout.feed.start(); await speedTimeout.reply(0, speedData()) +speedTimeout.feed.update(0, { value: 45 }); speedTimeout.timers.get(speedTimeout.feed.timer)() +assert.equal(speedTimeout.requests[1].options.signal.aborted, true) +await speedTimeout.reply(1, speedSaved) +assert.equal(speedTimeout.feed.data.slots[0].value, 30) +assert.equal(speedTimeout.feed.needsReload, true) + for (const status of [409, 503, 400]) { const failed = fixture(); failed.feed.start(); await failed.reply(0) failed.feed.update(0, { key: BOOKMARK }); await failed.reply(1, {}, status) @@ -93,4 +144,4 @@ assert.equal(rows.filter(({ row }) => row.page === "favorites").length, 1) const host = { state: {}, feed: { stop() {}, load() { assert.fail("Favorites is not a generic settings page") }, start() {} } } SettingsPage.methods.open.call(host, "favorites"); assert.equal(host.state.favoritesOpen, true) SettingsPage.methods.closeFavorites.call(host); assert.equal(host.state.section, "visual") -console.log("Favorites: exact slot edits, legacy preservation, no activation, races/errors/timeouts/auth, modern switch and Visual navigation passed") +console.log("Favorites: speed save/readback/validation, exact slot edits, legacy preservation, no activation, races/errors/timeouts/auth, modern switch and Visual navigation passed") diff --git a/openpilot/starpilot/galaxy/tests/test_favorites.py b/openpilot/starpilot/galaxy/tests/test_favorites.py index d9da4a094f..eb6f11f894 100644 --- a/openpilot/starpilot/galaxy/tests/test_favorites.py +++ b/openpilot/starpilot/galaxy/tests/test_favorites.py @@ -98,6 +98,31 @@ class TestFavoritesHttp(unittest.TestCase): self.assertEqual(self.request(payload={"revision": data["revision"], "slots": data["slots"]}, cookie=cookie)[0], 200) self.assertEqual(json.loads(Path(self.params.get_param_path(FAVORITE_SLOTS_PARAM)).read_text())[1], slots[1]) + def test_speed_edit_persists_and_preserves_other_slots_with_revision_protection(self): + slots = default_slots() + slots[0] = {"enabled": True, "show_onroad": False, "key": SET_SPEED, "label": "Cruise", "value": 30} + slots[1] = {"enabled": True, "show_onroad": True, "key": BOOKMARK, "label": "Mark"} + slots[2] = {"enabled": False, "show_onroad": False, "key": "OldUnsupportedSetting", "label": "Keep me"} + path = Path(self.params.get_param_path(FAVORITE_SLOTS_PARAM)) + path.write_text(json.dumps(slots)) + cookie = self.login() + _, data, _ = self.request(cookie=cookie) + data["slots"][0]["value"] = 45 + payload = {"revision": data["revision"], "slots": data["slots"]} + code, saved, _ = self.request(payload=payload, cookie=cookie) + self.assertEqual(code, 200) + expected = [{**slots[0], "value": 45}, *slots[1:]] + self.assertEqual(saved["slots"], expected) + self.assertEqual(json.loads(path.read_text()), expected) + code, reloaded, _ = self.request(cookie=cookie) + self.assertEqual(code, 200) + self.assertEqual(reloaded["slots"], expected) + self.assertNotEqual(reloaded["revision"], payload["revision"]) + payload["slots"][0]["value"] = 60 + self.assertEqual(self.request(payload=payload, cookie=cookie)[0], 409) + self.assertEqual(json.loads(path.read_text()), expected) + self.assertEqual(self.request("/api/favorites/action", {"key": SET_SPEED}, cookie)[0], 405) + if __name__ == "__main__": unittest.main() diff --git a/openpilot/starpilot/galaxy/web/js/favorites.js b/openpilot/starpilot/galaxy/web/js/favorites.js index 712f2dfed1..eeaeaab1f4 100644 --- a/openpilot/starpilot/galaxy/web/js/favorites.js +++ b/openpilot/starpilot/galaxy/web/js/favorites.js @@ -4,6 +4,10 @@ import { GxNotice } from "./notice.js" import { reactive } from "../vendor/vue/vue.esm-browser.js" import { GalaxySelect } from "./galaxy-select.js" +function validSpeedValue(value) { + return typeof value === "number" && Number.isFinite(value) && value >= 5 && value <= 145 +} + export function validFavorites(data) { return !!data && typeof data.revision === "string" && !!data.revision && typeof data.editable === "boolean" && typeof data.valid === "boolean" && Array.isArray(data.slots) && data.slots.length === 3 && @@ -34,7 +38,7 @@ export class FavoritesFeed { load() { return this.run() } update(index, patch) { if (!this.data?.editable || this.request || this.needsReload || !Number.isInteger(index) || index < 0 || index > 2 || - !patch || Object.keys(patch).some((key) => !["key", "label", "enabled", "show_onroad"].includes(key))) return + !patch || Object.keys(patch).some((key) => !["key", "label", "enabled", "show_onroad", "value"].includes(key))) return const slots = this.data.slots.map((slot) => ({ ...slot })) if (Object.hasOwn(patch, "key")) { const option = this.data.options.find((entry) => entry.key === patch.key) @@ -44,6 +48,7 @@ export class FavoritesFeed { if (!option) Object.assign(slots[index], { enabled: false, show_onroad: false }) } Object.assign(slots[index], patch) + if (Object.hasOwn(patch, "value") && (slots[index].key !== "__starpilot_controller_action__:set_speed" || !validSpeedValue(patch.value))) return return this.run({ revision: this.data.revision, slots }) } async run(body = null) { @@ -109,7 +114,7 @@ export const FavoritesPage = { }, speed(index, event) { const value = Number(event.target.value) - if (!Number.isFinite(value) || value < 5 || value > 145) return false + if (!validSpeedValue(value)) return false return this.feed.update(index, { value }) }, label(index, event) {