Skip to content

Commit e7b80b7

Browse files
maxime-desrochesmaxime-desroches
authored
params: auto type cast on put (#35810)
* start * fix * fix * more * more * more * fix * fix * [] * f * f * fix * lint * back * fix * yep * better msg * fix * fix * fix * fix * more * more
1 parent 26a9760 commit e7b80b7

File tree

19 files changed

+89
-73
lines changed

19 files changed

+89
-73
lines changed

‎common/params_keys.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ inline static std::unordered_map<std::string, ParamKeyAttributes> keys = {
7575
{"LastUpdateException", {CLEAR_ON_MANAGER_START, STRING}},
7676
{"LastUpdateTime", {PERSISTENT, TIME}},
7777
{"LiveDelay", {PERSISTENT, BYTES}},
78-
{"LiveParameters", {PERSISTENT, BYTES}},
78+
{"LiveParameters", {PERSISTENT, JSON}},
7979
{"LiveParametersV2", {PERSISTENT, BYTES}},
8080
{"LiveTorqueParameters", {PERSISTENT | DONT_LOG, BYTES}},
8181
{"LocationFilterInitialState", {PERSISTENT, BYTES}},
@@ -116,10 +116,10 @@ inline static std::unordered_map<std::string, ParamKeyAttributes> keys = {
116116
{"UpdateFailedCount", {CLEAR_ON_MANAGER_START, INT}},
117117
{"UpdaterAvailableBranches", {PERSISTENT, STRING}},
118118
{"UpdaterCurrentDescription", {CLEAR_ON_MANAGER_START, STRING}},
119-
{"UpdaterCurrentReleaseNotes", {CLEAR_ON_MANAGER_START, STRING}},
119+
{"UpdaterCurrentReleaseNotes", {CLEAR_ON_MANAGER_START, BYTES}},
120120
{"UpdaterFetchAvailable", {CLEAR_ON_MANAGER_START, BOOL}},
121121
{"UpdaterNewDescription", {CLEAR_ON_MANAGER_START, STRING}},
122-
{"UpdaterNewReleaseNotes", {CLEAR_ON_MANAGER_START, STRING}},
122+
{"UpdaterNewReleaseNotes", {CLEAR_ON_MANAGER_START, BYTES}},
123123
{"UpdaterState", {CLEAR_ON_MANAGER_START, STRING}},
124124
{"UpdaterTargetBranch", {CLEAR_ON_MANAGER_START, STRING}},
125125
{"UpdaterLastFetchTime", {PERSISTENT, TIME}},

‎common/params_pyx.pyx‎

Lines changed: 48 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
11
# distutils: language = c++
22
# cython: language_level = 3
3+
import builtins
34
import datetime
45
import json
56
from libcpp cimport bool
67
from libcpp.string cimport string
78
from libcpp.vector cimport vector
89
from libcpp.optional cimport optional
910

11+
from openpilot.common.swaglog import cloudlog
12+
1013
cdef extern from "common/params.h":
1114
cpdef enum ParamKeyFlag:
1215
PERSISTENT
@@ -42,6 +45,25 @@ cdef extern from "common/params.h":
4245
void clearAll(ParamKeyFlag)
4346
vector[string] allKeys()
4447

48+
PYTHON_2_CPP = {
49+
(str, STRING): lambda v: v,
50+
(builtins.bool, BOOL): lambda v: "1" if v else "0",
51+
(int, INT): str,
52+
(float, FLOAT): str,
53+
(datetime.datetime, TIME): lambda v: v.isoformat(),
54+
(dict, JSON): json.dumps,
55+
(list, JSON): json.dumps,
56+
(bytes, BYTES): lambda v: v,
57+
}
58+
CPP_2_PYTHON = {
59+
STRING: lambda v: v.decode("utf-8"),
60+
BOOL: lambda v: v == b"1",
61+
INT: int,
62+
FLOAT: float,
63+
TIME: lambda v: datetime.datetime.fromisoformat(v.decode("utf-8")),
64+
JSON: json.loads,
65+
BYTES: lambda v: v,
66+
}
4567

4668
def ensure_bytes(v):
4769
return v.encode() if isinstance(v, str) else v
@@ -74,45 +96,38 @@ cdef class Params:
7496
raise UnknownKeyName(key)
7597
return key
7698

77-
def cast(self, t, value, default):
99+
def python2cpp(self, proposed_type, expected_type, value, key):
100+
cast = PYTHON_2_CPP.get((proposed_type, expected_type))
101+
if cast:
102+
return cast(value)
103+
raise TypeError(f"Type mismatch while writing param {key}: {proposed_type=} {expected_type=} {value=}")
104+
105+
def cpp2python(self, t, value, default, key):
78106
if value is None:
79107
return None
80108
try:
81-
if t == STRING:
82-
return value.decode("utf-8")
83-
elif t == BOOL:
84-
return value == b"1"
85-
elif t == INT:
86-
return int(value)
87-
elif t == FLOAT:
88-
return float(value)
89-
elif t == TIME:
90-
return datetime.datetime.fromisoformat(value.decode("utf-8"))
91-
elif t == JSON:
92-
return json.loads(value)
93-
elif t == BYTES:
94-
return value
95-
else:
96-
raise TypeError()
97-
except (TypeError, ValueError):
98-
return self.cast(t, default, None)
109+
return CPP_2_PYTHON[t](value)
110+
except (KeyError, TypeError, ValueError):
111+
cloudlog.warning(f"Failed to cast param {key} with {value=} from type {t=}")
112+
return self.cpp2python(t, default, None, key)
99113

100114
def get(self, key, bool block=False, bool return_default=False):
101115
cdef string k = self.check_key(key)
102116
cdef ParamKeyType t = self.p.getKeyType(k)
117+
cdef optional[string] default = self.p.getKeyDefaultValue(k)
103118
cdef string val
104119
with nogil:
105120
val = self.p.get(k, block)
106121

107-
default_val = self.get_default_value(k) if return_default else None
122+
default_val = (default.value() if default.has_value() else None) if return_default else None
108123
if val == b"":
109124
if block:
110125
# If we got no value while running in blocked mode
111126
# it means we got an interrupt while waiting
112127
raise KeyboardInterrupt
113128
else:
114-
return self.cast(t, default_val, None)
115-
return self.cast(t, val, default_val)
129+
return self.cpp2python(t, default_val, None, key)
130+
return self.cpp2python(t, val, default_val, key)
116131

117132
def get_bool(self, key, bool block=False):
118133
cdef string k = self.check_key(key)
@@ -121,6 +136,11 @@ cdef class Params:
121136
r = self.p.getBool(k, block)
122137
return r
123138

139+
def _put_cast(self, key, dat):
140+
cdef string k = self.check_key(key)
141+
cdef ParamKeyType t = self.p.getKeyType(k)
142+
return ensure_bytes(self.python2cpp(type(dat), t, dat, key))
143+
124144
def put(self, key, dat):
125145
"""
126146
Warning: This function blocks until the param is written to disk!
@@ -129,7 +149,7 @@ cdef class Params:
129149
in general try to avoid writing params as much as possible.
130150
"""
131151
cdef string k = self.check_key(key)
132-
cdef string dat_bytes = ensure_bytes(dat)
152+
cdef string dat_bytes = self._put_cast(key, dat)
133153
with nogil:
134154
self.p.put(k, dat_bytes)
135155

@@ -140,7 +160,7 @@ cdef class Params:
140160

141161
def put_nonblocking(self, key, dat):
142162
cdef string k = self.check_key(key)
143-
cdef string dat_bytes = ensure_bytes(dat)
163+
cdef string dat_bytes = self._put_cast(key, dat)
144164
with nogil:
145165
self.p.putNonBlocking(k, dat_bytes)
146166

@@ -165,5 +185,7 @@ cdef class Params:
165185
return self.p.allKeys()
166186

167187
def get_default_value(self, key):
168-
cdef optional[string] default = self.p.getKeyDefaultValue(self.check_key(key))
169-
return default.value() if default.has_value() else None
188+
cdef string k = self.check_key(key)
189+
cdef ParamKeyType t = self.p.getKeyType(k)
190+
cdef optional[string] default = self.p.getKeyDefaultValue(k)
191+
return self.cpp2python(t, default.value(), None, key) if default.has_value() else None

‎common/tests/test_params.py‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import pytest
22
import datetime
3-
import json
43
import os
54
import threading
65
import time
@@ -22,7 +21,7 @@ def test_params_non_ascii(self):
2221
assert self.params.get("CarParams") == st
2322

2423
def test_params_get_cleared_manager_start(self):
25-
self.params.put("CarParams", "test")
24+
self.params.put("CarParams", b"test")
2625
self.params.put("DongleId", "cb38263377b873ee")
2726
assert self.params.get("CarParams") == b"test"
2827

@@ -45,10 +44,10 @@ def test_params_two_things(self):
4544
def test_params_get_block(self):
4645
def _delayed_writer():
4746
time.sleep(0.1)
48-
self.params.put("CarParams", "test")
47+
self.params.put("CarParams", b"test")
4948
threading.Thread(target=_delayed_writer).start()
5049
assert self.params.get("CarParams") is None
51-
assert self.params.get("CarParams", True) == b"test"
50+
assert self.params.get("CarParams", block=True) == b"test"
5251

5352
def test_params_unknown_key_fails(self):
5453
with pytest.raises(UnknownKeyName):
@@ -78,17 +77,17 @@ def test_get_bool(self):
7877
self.params.put_bool("IsMetric", False)
7978
assert not self.params.get_bool("IsMetric")
8079

81-
self.params.put("IsMetric", "1")
80+
self.params.put("IsMetric", True)
8281
assert self.params.get_bool("IsMetric")
8382

84-
self.params.put("IsMetric", "0")
83+
self.params.put("IsMetric", False)
8584
assert not self.params.get_bool("IsMetric")
8685

8786
def test_put_non_blocking_with_get_block(self):
8887
q = Params()
8988
def _delayed_writer():
9089
time.sleep(0.1)
91-
Params().put_nonblocking("CarParams", "test")
90+
Params().put_nonblocking("CarParams", b"test")
9291
threading.Thread(target=_delayed_writer).start()
9392
assert q.get("CarParams") is None
9493
assert q.get("CarParams", True) == b"test"
@@ -124,19 +123,19 @@ def test_params_default_value(self):
124123

125124
def test_params_get_type(self):
126125
# json
127-
self.params.put("ApiCache_FirehoseStats", json.dumps({"a": 0}))
126+
self.params.put("ApiCache_FirehoseStats", {"a": 0})
128127
assert self.params.get("ApiCache_FirehoseStats") == {"a": 0}
129128

130129
# int
131-
self.params.put("BootCount", str(1441))
130+
self.params.put("BootCount", 1441)
132131
assert self.params.get("BootCount") == 1441
133132

134133
# bool
135-
self.params.put("AdbEnabled", "1")
134+
self.params.put("AdbEnabled", True)
136135
assert self.params.get("AdbEnabled")
137136
assert isinstance(self.params.get("AdbEnabled"), bool)
138137

139138
# time
140139
now = datetime.datetime.now(datetime.UTC)
141-
self.params.put("InstallDate", str(now))
140+
self.params.put("InstallDate", now)
142141
assert self.params.get("InstallDate") == now

‎selfdrive/locationd/paramsd.py‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
#!/usr/bin/env python3
22
import os
3-
import json
43
import numpy as np
54
import capnp
65

@@ -207,12 +206,11 @@ def migrate_cached_vehicle_params_if_needed(params: Params):
207206
return
208207

209208
try:
210-
last_parameters_dict = json.loads(last_parameters_data_old)
211209
last_parameters_msg = messaging.new_message('liveParameters')
212210
last_parameters_msg.liveParameters.valid = True
213-
last_parameters_msg.liveParameters.steerRatio = last_parameters_dict['steerRatio']
214-
last_parameters_msg.liveParameters.stiffnessFactor = last_parameters_dict['stiffnessFactor']
215-
last_parameters_msg.liveParameters.angleOffsetAverageDeg = last_parameters_dict['angleOffsetAverageDeg']
211+
last_parameters_msg.liveParameters.steerRatio = last_parameters_data_old['steerRatio']
212+
last_parameters_msg.liveParameters.stiffnessFactor = last_parameters_data_old['stiffnessFactor']
213+
last_parameters_msg.liveParameters.angleOffsetAverageDeg = last_parameters_data_old['angleOffsetAverageDeg']
216214
params.put("LiveParametersV2", last_parameters_msg.to_bytes())
217215
except Exception as e:
218216
cloudlog.error(f"Failed to perform parameter migration: {e}")

‎selfdrive/locationd/test/test_paramsd.py‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import random
22
import numpy as np
3-
import json
43

54
from cereal import messaging
65
from openpilot.selfdrive.locationd.paramsd import retrieve_initial_vehicle_params, migrate_cached_vehicle_params_if_needed
@@ -47,7 +46,7 @@ def test_read_saved_params_old_format(self):
4746
CP = next(m for m in lr if m.which() == "carParams").carParams
4847

4948
msg = get_random_live_parameters(CP)
50-
params.put("LiveParameters", json.dumps(msg.liveParameters.to_dict()))
49+
params.put("LiveParameters", msg.liveParameters.to_dict())
5150
params.put("CarParamsPrevRoute", CP.as_builder().to_bytes())
5251
params.remove("LiveParametersV2")
5352

@@ -60,7 +59,7 @@ def test_read_saved_params_old_format(self):
6059

6160
def test_read_saved_params_corrupted_old_format(self):
6261
params = Params()
63-
params.put("LiveParameters", b'\x00\x00\x02\x00\x01\x00:F\xde\xed\xae;')
62+
params.put("LiveParameters", {})
6463
params.remove("LiveParametersV2")
6564

6665
migrate_cached_vehicle_params_if_needed(params)

‎selfdrive/selfdrived/alertmanager.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ def set_offroad_alert(alert: str, show_alert: bool, extra_text: str = None) -> N
1717
if show_alert:
1818
a = copy.copy(OFFROAD_ALERTS[alert])
1919
a['extra'] = extra_text or ''
20-
Params().put(alert, json.dumps(a))
20+
Params().put(alert, a)
2121
else:
2222
Params().remove(alert)
2323

‎selfdrive/selfdrived/selfdrived.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,7 @@ def update_events(self, CS):
407407
if self.CP.openpilotLongitudinalControl:
408408
if any(not be.pressed and be.type == ButtonType.gapAdjustCruise for be in CS.buttonEvents):
409409
self.personality = (self.personality - 1) % 3
410-
self.params.put_nonblocking('LongitudinalPersonality', str(self.personality))
410+
self.params.put_nonblocking('LongitudinalPersonality', self.personality)
411411
self.events.add(EventName.personalityChanged)
412412

413413
def data_sample(self):

‎selfdrive/ui/layouts/settings/firehose.py‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import pyray as rl
2-
import json
32
import time
43
import threading
54

@@ -169,7 +168,7 @@ def _fetch_firehose_stats(self):
169168
if response.status_code == 200:
170169
data = response.json()
171170
self.segment_count = data.get("firehose", 0)
172-
self.params.put(self.PARAM_KEY, json.dumps(data))
171+
self.params.put(self.PARAM_KEY, data)
173172
except Exception as e:
174173
cloudlog.error(f"Failed to fetch firehose stats: {e}")
175174

‎selfdrive/ui/layouts/settings/toggles.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,4 +92,4 @@ def _render(self, rect):
9292
self._scroller.render(rect)
9393

9494
def _set_longitudinal_personality(self, button_index: int):
95-
self._params.put("LongitudinalPersonality", str(button_index))
95+
self._params.put("LongitudinalPersonality", button_index)

‎selfdrive/ui/lib/prime_state.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ def set_type(self, prime_type: PrimeType) -> None:
6363
with self._lock:
6464
if prime_type != self.prime_type:
6565
self.prime_type = prime_type
66-
self._params.put("PrimeType", str(int(prime_type)))
66+
self._params.put("PrimeType", int(prime_type))
6767
cloudlog.info(f"Prime type updated to {prime_type}")
6868

6969
def _worker_thread(self) -> None:

0 commit comments

Comments
 (0)