Remember the rider, not only the hardware
`RiderConfig` lived in `RideInputs` and nowhere else, and nothing in the UI ever called `set_rider_config`. So every ride was ridden as the struct's own default — a 105 kg rider on an 8 kg bike — with no way to say otherwise short of editing the source. Mass is not a preference: it sets the speed a given power produces, the ETA that follows from it, the calorie estimate, and how a 6% ramp feels. FR-7.4 is a Must, and a command the UI never calls does not satisfy it. - settings.rs persists rider config, safety limits and display preferences to app_data_dir()/settings.json, written atomically and read back before the first tick, so no snapshot is ever computed against the default. Advisory like known.rs: an unreadable file costs the rider their setup, never their ride. - A stored file is refused *whole* if it fails the same checks the commands apply. It may predate a tightened bound or have been edited by hand, and a zero mass reaching the engine divides by itself on the next tick. - The commands validate with instructions rather than codes — "CdA must be between 0.1 and 1.5 m² — a road position is about 0.32" — because this is now a form a rider fills in, not a struct only I ever touched. - Preferences (FTP, maximum heart rate, units) are Tauri-side, not in `RiderConfig`. None of it reaches the physics, and crates/core is the frozen contract the engine and the FIT writer share. Zero is a real answer for both references and means "no zones", not "unset and guessed at". - SettingsScreen commits on field-exit and reseats every input from what Rust returned, so a rejected value can never sit on screen looking accepted. Weight, FTP and units are on top; the eight settings with a defensible default are folded away. - Reachable on `,` from any screen, returning to whichever screen opened it. Setup swallows the ride controls while it is up — a stray arrow key while reading the form must not trim the gradient of a ride happening behind it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
@@ -441,10 +441,19 @@ pub fn set_rider_config(
|
||||
state: State<'_, AppState>,
|
||||
config: RiderConfig,
|
||||
) -> Cmd<RiderConfig> {
|
||||
if config.rider_kg <= 20.0 || config.bike_kg <= 0.0 {
|
||||
return Err("Rider and bike mass must be positive and realistic".into());
|
||||
// Every bound refused here is one that makes the engine produce nonsense
|
||||
// rather than something merely unusual, and each refusal says what a
|
||||
// workable value looks like — this is now a form a rider fills in, not a
|
||||
// struct only the developer ever touched.
|
||||
crate::settings::validate_rider(&config)?;
|
||||
{
|
||||
let mut inner = state.lock();
|
||||
inner.inputs.rider = config;
|
||||
// Written down immediately. A setup that lasts only as long as the
|
||||
// process is what left every rider on the 105 kg default (FR-7.4).
|
||||
let inner = &*inner;
|
||||
inner.settings.save(&inner.inputs.rider, &inner.inputs.limits);
|
||||
}
|
||||
state.lock().inputs.rider = config;
|
||||
emit_ride_state(&app);
|
||||
Ok(config)
|
||||
}
|
||||
@@ -460,14 +469,40 @@ pub fn set_safety_limits(
|
||||
state: State<'_, AppState>,
|
||||
limits: SafetyLimits,
|
||||
) -> Cmd<SafetyLimits> {
|
||||
if limits.min_gradient_pct >= limits.max_gradient_pct {
|
||||
return Err("Gradient limits are inverted".into());
|
||||
crate::settings::validate_limits(&limits)?;
|
||||
{
|
||||
let mut inner = state.lock();
|
||||
inner.inputs.limits = limits;
|
||||
let inner = &*inner;
|
||||
inner.settings.save(&inner.inputs.rider, &inner.inputs.limits);
|
||||
}
|
||||
state.lock().inputs.limits = limits;
|
||||
emit_ride_state(&app);
|
||||
Ok(limits)
|
||||
}
|
||||
|
||||
/// Display preferences: FTP, maximum heart rate, units (§4.3).
|
||||
///
|
||||
/// Separate from [`rider_config`] because nothing here reaches the physics —
|
||||
/// these decide how a number is drawn, not what it is. They are stored in the
|
||||
/// same file because that is where the rider expects to find them.
|
||||
#[tauri::command]
|
||||
pub fn preferences(state: State<'_, AppState>) -> crate::settings::Preferences {
|
||||
state.lock().settings.prefs
|
||||
}
|
||||
|
||||
#[tauri::command]
|
||||
pub fn set_preferences(
|
||||
state: State<'_, AppState>,
|
||||
prefs: crate::settings::Preferences,
|
||||
) -> Cmd<crate::settings::Preferences> {
|
||||
crate::settings::validate_prefs(&prefs)?;
|
||||
let mut inner = state.lock();
|
||||
inner.settings.prefs = prefs;
|
||||
let inner = &*inner;
|
||||
inner.settings.save(&inner.inputs.rider, &inner.inputs.limits);
|
||||
Ok(prefs)
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Profiles (§5.5, §5.6)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
@@ -15,6 +15,7 @@ pub mod devices;
|
||||
pub mod events;
|
||||
pub mod heart_rate;
|
||||
pub mod known;
|
||||
pub mod settings;
|
||||
pub mod profile_view;
|
||||
pub mod recording;
|
||||
pub mod samples;
|
||||
@@ -98,6 +99,8 @@ pub fn run() {
|
||||
commands::set_rider_config,
|
||||
commands::safety_limits,
|
||||
commands::set_safety_limits,
|
||||
commands::preferences,
|
||||
commands::set_preferences,
|
||||
// profiles
|
||||
commands::load_profile_from_path,
|
||||
commands::load_profile_from_text,
|
||||
@@ -130,6 +133,20 @@ pub fn run() {
|
||||
Ok(path) => handle.state::<AppState>().lock().devices.attach_store(path),
|
||||
Err(e) => tracing::warn!(error = %e, "remembered devices unavailable"),
|
||||
}
|
||||
// FR-7.4: the rider's mass, bike and drag, before the first tick
|
||||
// reads them. Restored ahead of the ride loop starting so no
|
||||
// snapshot is ever computed against the 105 kg default the struct
|
||||
// falls back to.
|
||||
match settings::store_path(&handle) {
|
||||
Ok(path) => {
|
||||
let state = handle.state::<AppState>();
|
||||
let mut inner = state.lock();
|
||||
let inner = &mut *inner;
|
||||
let (rider, limits) = (&mut inner.inputs.rider, &mut inner.inputs.limits);
|
||||
inner.settings.attach(path, rider, limits);
|
||||
}
|
||||
Err(e) => tracing::warn!(error = %e, "rider settings unavailable"),
|
||||
}
|
||||
// NFR-7: scanning starts immediately, not on a user click.
|
||||
handle.state::<AppState>().lock().devices.start_scan();
|
||||
state::spawn_ride_loop(handle.clone());
|
||||
|
||||
@@ -0,0 +1,343 @@
|
||||
//! Rider setup, remembered across launches (FR-7.4).
|
||||
//!
|
||||
//! Two things were wrong before this module existed, and they were the same
|
||||
//! thing twice.
|
||||
//!
|
||||
//! `RiderConfig` lived only in [`crate::backend::RideInputs`], which meant it
|
||||
//! lived exactly as long as the process — and nothing in the UI ever called
|
||||
//! `set_rider_config`, so in practice every ride was ridden as a **105 kg
|
||||
//! rider on an 8 kg bike**, the struct's own defaults. Mass is not a cosmetic
|
||||
//! setting: it sets the speed a given power produces, the ETA that follows from
|
||||
//! it, the calorie estimate, and how a 6% ramp feels. A 62 kg rider was being
|
||||
//! shown somebody else's ride.
|
||||
//!
|
||||
//! ```text
|
||||
//! app_data_dir()/settings.json
|
||||
//! { "version": 1, "rider": {…}, "limits": {…}, "prefs": {…} }
|
||||
//! ```
|
||||
//!
|
||||
//! Written whole on every change — it is three small structs, and settings are
|
||||
//! changed by hand at human speed, so this never lands in the ride loop's path.
|
||||
//! Like [`crate::known`] it is *advisory*: an unreadable file costs the rider
|
||||
//! their setup, never their ride, so every failure is logged and swallowed.
|
||||
//!
|
||||
//! ## Why `prefs` is here and not in `RiderConfig`
|
||||
//!
|
||||
//! FTP, maximum heart rate and the unit system change nothing about the
|
||||
//! physics — they decide how a number is *drawn*. `bikecontrol_core::types` is
|
||||
//! the frozen contract the engine and the FIT writer share, and a display
|
||||
//! preference has no business in it. The file keeps them side by side because
|
||||
//! that is where the rider expects to find them; the types stay apart.
|
||||
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
use bikecontrol_core::types::{RiderConfig, SafetyLimits};
|
||||
use serde::{Deserialize, Serialize};
|
||||
use tauri::{AppHandle, Manager};
|
||||
|
||||
/// Bumped only if the shape changes incompatibly. An older file with a version
|
||||
/// we do not know is discarded rather than guessed at.
|
||||
const VERSION: u32 = 1;
|
||||
const FILE: &str = "settings.json";
|
||||
|
||||
/// Which units the rider reads. Everything is *stored* and *recorded* in SI
|
||||
/// regardless — this is the last conversion before the glass, so a FIT file
|
||||
/// never depends on what the screen was set to.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub enum Units {
|
||||
#[default]
|
||||
Metric,
|
||||
Imperial,
|
||||
}
|
||||
|
||||
/// Display preferences. Not physics — see the module note.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
|
||||
#[serde(rename_all = "camelCase", default)]
|
||||
pub struct Preferences {
|
||||
/// Functional threshold power, watts. The reference every power zone is a
|
||||
/// fraction of; zero means the rider has not set one and zones are not
|
||||
/// drawn at all, which is honest — a zone against a guessed FTP is worse
|
||||
/// than no zone.
|
||||
pub ftp_w: u16,
|
||||
/// Maximum heart rate, bpm. Same contract: zero means no HR zones.
|
||||
pub max_hr_bpm: u16,
|
||||
pub units: Units,
|
||||
}
|
||||
|
||||
impl Default for Preferences {
|
||||
fn default() -> Self {
|
||||
Self {
|
||||
// Deliberately no default FTP: an invented threshold would colour
|
||||
// every ride wrong and look authoritative doing it.
|
||||
ftp_w: 0,
|
||||
max_hr_bpm: 0,
|
||||
units: Units::Metric,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// The file on disk.
|
||||
#[derive(Debug, Default, Serialize, Deserialize)]
|
||||
#[serde(rename_all = "camelCase", default)]
|
||||
struct Stored {
|
||||
version: u32,
|
||||
rider: RiderConfig,
|
||||
limits: SafetyLimits,
|
||||
prefs: Preferences,
|
||||
}
|
||||
|
||||
/// The rider's setup, plus where to write it.
|
||||
#[derive(Debug, Default)]
|
||||
pub struct Settings {
|
||||
pub prefs: Preferences,
|
||||
/// `None` before [`Settings::attach`] — `AppState::new` runs before there
|
||||
/// is an `AppHandle` to ask for a data directory, so the first moments of
|
||||
/// the process are in-memory only.
|
||||
path: Option<PathBuf>,
|
||||
}
|
||||
|
||||
impl Settings {
|
||||
/// Point at the file and read it back over the defaults already in
|
||||
/// `rider` and `limits`.
|
||||
///
|
||||
/// Applied by `&mut` rather than returned because a partial application is
|
||||
/// the one outcome that must not be possible: the rider's mass and the
|
||||
/// safety clamps that bound what can be sent to the trainer come from the
|
||||
/// same file and are adopted in the same breath.
|
||||
pub fn attach(&mut self, path: PathBuf, rider: &mut RiderConfig, limits: &mut SafetyLimits) {
|
||||
self.path = Some(path.clone());
|
||||
let text = match std::fs::read_to_string(&path) {
|
||||
Ok(t) => t,
|
||||
Err(e) if e.kind() == std::io::ErrorKind::NotFound => return,
|
||||
Err(e) => {
|
||||
tracing::warn!(path = %path.display(), error = %e, "could not read settings");
|
||||
return;
|
||||
}
|
||||
};
|
||||
let stored: Stored = match serde_json::from_str(&text) {
|
||||
Ok(s) => s,
|
||||
Err(e) => {
|
||||
tracing::warn!(path = %path.display(), error = %e, "settings are unreadable; using defaults");
|
||||
return;
|
||||
}
|
||||
};
|
||||
if stored.version != VERSION {
|
||||
tracing::warn!(
|
||||
found = stored.version,
|
||||
expected = VERSION,
|
||||
"settings are from another version; using defaults"
|
||||
);
|
||||
return;
|
||||
}
|
||||
// A stored file that fails the same checks the commands apply is not
|
||||
// trusted just because it is on disk — it may predate a tightened
|
||||
// bound, or have been edited by hand.
|
||||
if validate_rider(&stored.rider).is_err() || validate_limits(&stored.limits).is_err() {
|
||||
tracing::warn!("stored settings are out of range; using defaults");
|
||||
return;
|
||||
}
|
||||
*rider = stored.rider;
|
||||
*limits = stored.limits;
|
||||
self.prefs = stored.prefs;
|
||||
tracing::info!(rider_kg = stored.rider.rider_kg, "settings restored");
|
||||
}
|
||||
|
||||
/// Write the file, atomically. A half-written `settings.json` is discarded
|
||||
/// whole on the next launch, which would silently put the rider back on a
|
||||
/// 105 kg default — the exact failure this module exists to prevent.
|
||||
pub fn save(&self, rider: &RiderConfig, limits: &SafetyLimits) {
|
||||
let Some(path) = &self.path else {
|
||||
return;
|
||||
};
|
||||
let stored = Stored {
|
||||
version: VERSION,
|
||||
rider: *rider,
|
||||
limits: *limits,
|
||||
prefs: self.prefs,
|
||||
};
|
||||
if let Err(e) = write_atomic(path, &stored) {
|
||||
tracing::warn!(path = %path.display(), error = %e, "could not save settings");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Bounds worth refusing, with the reason a rider can act on.
|
||||
///
|
||||
/// These are not taste. Each one is a value that makes the ride engine produce
|
||||
/// nonsense rather than merely something unusual — a zero mass divides, a zero
|
||||
/// wheel circumference divides, an efficiency above 1 invents power.
|
||||
pub fn validate_rider(c: &RiderConfig) -> Result<(), String> {
|
||||
let check = |ok: bool, msg: &str| if ok { Ok(()) } else { Err(msg.to_string()) };
|
||||
check(
|
||||
(20.0..=250.0).contains(&c.rider_kg),
|
||||
"Rider mass must be between 20 and 250 kg.",
|
||||
)?;
|
||||
check(
|
||||
(1.0..=50.0).contains(&c.bike_kg),
|
||||
"Bike mass must be between 1 and 50 kg.",
|
||||
)?;
|
||||
check(
|
||||
(0.0005..=0.05).contains(&c.crr),
|
||||
"Rolling resistance is typically 0.002–0.010 for road tyres.",
|
||||
)?;
|
||||
check(
|
||||
(0.1..=1.5).contains(&c.cda),
|
||||
"CdA must be between 0.1 and 1.5 m² — a road position is about 0.32.",
|
||||
)?;
|
||||
check(
|
||||
(0.5..=1.0).contains(&c.drivetrain_efficiency),
|
||||
"Drivetrain efficiency is a fraction between 0.5 and 1.0 — about 0.97 for a clean chain.",
|
||||
)?;
|
||||
check(
|
||||
(0.5..=1.6).contains(&c.air_density),
|
||||
"Air density must be between 0.5 and 1.6 kg/m³ — sea level is 1.225.",
|
||||
)?;
|
||||
check(
|
||||
(0.5..=3.5).contains(&c.wheel_circumference_m),
|
||||
"Wheel circumference must be between 0.5 and 3.5 m — a 700×25 is about 2.1.",
|
||||
)?;
|
||||
check(
|
||||
(0.1..=0.25).contains(&c.crank_length_m),
|
||||
"Crank length must be between 0.10 and 0.25 m — road cranks are 0.170–0.175.",
|
||||
)?;
|
||||
check(
|
||||
(0.5..=20.0).contains(&c.physical_development_m),
|
||||
"Physical development must be between 0.5 and 20 m per crank revolution.",
|
||||
)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub fn validate_limits(l: &SafetyLimits) -> Result<(), String> {
|
||||
if l.min_gradient_pct >= l.max_gradient_pct {
|
||||
return Err("Gradient limits are inverted.".into());
|
||||
}
|
||||
if l.min_resistance >= l.max_resistance {
|
||||
return Err("Resistance limits are inverted.".into());
|
||||
}
|
||||
if l.min_power_w >= l.max_power_w {
|
||||
return Err("Power limits are inverted.".into());
|
||||
}
|
||||
if l.max_power_w > 2000 {
|
||||
return Err("Maximum power above 2000 W is not a limit, it is a hazard.".into());
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub fn validate_prefs(p: &Preferences) -> Result<(), String> {
|
||||
// Zero is the "not set" case for both, and must stay reachable: a rider who
|
||||
// does not know their FTP is better served by no zones than by a guess.
|
||||
if p.ftp_w != 0 && !(50..=600).contains(&p.ftp_w) {
|
||||
return Err("FTP must be between 50 and 600 W, or 0 for no zones.".into());
|
||||
}
|
||||
if p.max_hr_bpm != 0 && !(100..=230).contains(&p.max_hr_bpm) {
|
||||
return Err("Maximum heart rate must be between 100 and 230 bpm, or 0 for no zones.".into());
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn write_atomic(path: &Path, stored: &Stored) -> std::io::Result<()> {
|
||||
let text = serde_json::to_string_pretty(stored)
|
||||
.map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))?;
|
||||
if let Some(dir) = path.parent() {
|
||||
std::fs::create_dir_all(dir)?;
|
||||
}
|
||||
let tmp = path.with_extension("json.tmp");
|
||||
std::fs::write(&tmp, text)?;
|
||||
std::fs::rename(&tmp, path)
|
||||
}
|
||||
|
||||
/// Beside the remembered devices and the recorded rides.
|
||||
pub fn store_path(app: &AppHandle) -> Result<PathBuf, String> {
|
||||
let dir = app
|
||||
.path()
|
||||
.app_data_dir()
|
||||
.map_err(|e| format!("no app data directory: {e}"))?;
|
||||
std::fs::create_dir_all(&dir)
|
||||
.map_err(|e| format!("could not create {}: {e}", dir.display()))?;
|
||||
Ok(dir.join(FILE))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
fn temp(name: &str) -> PathBuf {
|
||||
std::env::temp_dir().join(format!("bikecontrol-settings-test-{name}.json"))
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_rider_survives_the_process() {
|
||||
let path = temp("roundtrip");
|
||||
let _ = std::fs::remove_file(&path);
|
||||
|
||||
let mut settings = Settings::default();
|
||||
let mut rider = RiderConfig::default();
|
||||
let mut limits = SafetyLimits::default();
|
||||
settings.attach(path.clone(), &mut rider, &mut limits);
|
||||
rider.rider_kg = 62.0;
|
||||
settings.prefs.ftp_w = 240;
|
||||
settings.save(&rider, &limits);
|
||||
|
||||
let mut again = Settings::default();
|
||||
let mut rider2 = RiderConfig::default();
|
||||
let mut limits2 = SafetyLimits::default();
|
||||
again.attach(path.clone(), &mut rider2, &mut limits2);
|
||||
assert_eq!(rider2.rider_kg, 62.0);
|
||||
assert_eq!(again.prefs.ftp_w, 240);
|
||||
|
||||
let _ = std::fs::remove_file(&path);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_missing_file_leaves_the_defaults_alone() {
|
||||
let path = temp("missing");
|
||||
let _ = std::fs::remove_file(&path);
|
||||
let mut settings = Settings::default();
|
||||
let mut rider = RiderConfig::default();
|
||||
let mut limits = SafetyLimits::default();
|
||||
settings.attach(path, &mut rider, &mut limits);
|
||||
assert_eq!(rider.rider_kg, RiderConfig::default().rider_kg);
|
||||
assert_eq!(settings.prefs, Preferences::default());
|
||||
}
|
||||
|
||||
/// A file someone edited by hand must not be able to put a zero mass into
|
||||
/// the engine, which would divide by it on the next tick.
|
||||
#[test]
|
||||
fn an_out_of_range_file_is_refused_whole() {
|
||||
let path = temp("nonsense");
|
||||
let stored = Stored {
|
||||
version: VERSION,
|
||||
rider: RiderConfig {
|
||||
rider_kg: 0.0,
|
||||
..RiderConfig::default()
|
||||
},
|
||||
limits: SafetyLimits::default(),
|
||||
prefs: Preferences {
|
||||
ftp_w: 300,
|
||||
..Preferences::default()
|
||||
},
|
||||
};
|
||||
write_atomic(&path, &stored).unwrap();
|
||||
|
||||
let mut settings = Settings::default();
|
||||
let mut rider = RiderConfig::default();
|
||||
let mut limits = SafetyLimits::default();
|
||||
settings.attach(path.clone(), &mut rider, &mut limits);
|
||||
assert_eq!(rider.rider_kg, RiderConfig::default().rider_kg);
|
||||
// Refused whole: the preferences in the same file do not sneak through.
|
||||
assert_eq!(settings.prefs.ftp_w, 0);
|
||||
|
||||
let _ = std::fs::remove_file(&path);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn zero_means_no_zones_rather_than_an_invalid_ftp() {
|
||||
assert!(validate_prefs(&Preferences::default()).is_ok());
|
||||
assert!(validate_prefs(&Preferences {
|
||||
ftp_w: 20,
|
||||
..Preferences::default()
|
||||
})
|
||||
.is_err());
|
||||
}
|
||||
}
|
||||
@@ -54,6 +54,8 @@ pub struct Inner {
|
||||
/// Rides rebuilt from an orphaned journal at startup (FR-8.4), held until
|
||||
/// the webview asks for them.
|
||||
pub recovered: Vec<Recovered>,
|
||||
/// The rider's own setup, and where it is written down (FR-7.4).
|
||||
pub settings: crate::settings::Settings,
|
||||
lap_start_ms: u64,
|
||||
lap_start_m: f64,
|
||||
lap_power_sum: f64,
|
||||
@@ -104,6 +106,7 @@ impl Inner {
|
||||
laps: Vec::new(),
|
||||
last_summary: None,
|
||||
recovered: Vec::new(),
|
||||
settings: crate::settings::Settings::default(),
|
||||
lap_start_ms: 0,
|
||||
lap_start_m: 0.0,
|
||||
lap_power_sum: 0.0,
|
||||
|
||||
Reference in New Issue
Block a user