Set speed favorite fix

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()

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")

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()

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) {