fix(qbzd): serialize volume canonically in JSON responses

cc
This commit is contained in:
vicrodh committed 2026-07-14 16:49:20 -06:00
1 parent e700084f0b
commit 393a6e01a7
3 files changed
+103 -7

No files matched your search

+22
View File
@@ -306,6 +306,18 @@ fn constant_time_eq(a: &[u8], b: &[u8]) -> bool {
diff == 0
}
/// Canonical JSON number for a volume level (0.0-1.0). `serde_json::Value`
/// backs f32 via `Number::from_f32`, which stores the value as `f as f64` —
/// the f32→f64 widening turns `0.8f32` into `0.800000011920929` on the wire.
/// 02-cli-and-api.md §2.2/§3.3.4 document plain `0.8`, and `--json` is the
/// frozen machine contract ("scripts parse this"), so every volume-bearing
/// response routes through this instead of a bare `json!(v)`. 3 decimals is
/// plenty of precision for a 0.0-1.0 level.
pub(crate) fn canon_volume(v: f32) -> serde_json::Value {
let rounded = (v as f64 * 1000.0).round() / 1000.0;
serde_json::json!(rounded)
}
/// A 2xx JSON response. `pub(crate)` so the per-route handlers in `status.rs`
/// share the exact same envelope framing.
pub(crate) fn json(status: u16, body: serde_json::Value) -> Response<Cursor<Vec<u8>>> {
@@ -415,6 +427,16 @@ mod tests {
assert!(access_gate(false, "GET", "/api/ping", Some("Bearer nope"), tok).is_none());
}
#[test]
fn canon_volume_pins_0_8_exactly_no_f32_widening() {
// `serde_json::Number::from_f32` widens f32→f64 (`0.8f32` would
// serialize raw as `0.800000011920929`); `canon_volume` must not.
assert_eq!(serde_json::to_string(&canon_volume(0.8f32)).unwrap(), "0.8");
assert_eq!(serde_json::to_string(&canon_volume(1.0f32)).unwrap(), "1.0");
assert_eq!(serde_json::to_string(&canon_volume(0.0f32)).unwrap(), "0.0");
assert_eq!(serde_json::to_string(&canon_volume(0.75f32)).unwrap(), "0.75");
}
#[test]
fn error_envelope_is_the_nested_shape_with_code() {
// 02 §3.1.3 — the on-the-wire shape the CLI reads via `error.code`.
+57 -3
View File
@@ -32,7 +32,7 @@ use tiny_http::Response;
use crate::state::AuthState;
use super::{err_json, json, ApiState};
use super::{canon_volume, err_json, json, ApiState};
/// `GET /api/now-playing` (02 §3.3.4). `playback` is the serialized
/// `PlaybackEvent` (qbz-player/src/player/mod.rs:925) with `shuffle`/`repeat`
@@ -62,6 +62,9 @@ pub fn now_playing(state: &ApiState) -> Response<Cursor<Vec<u8>>> {
if let Value::Object(map) = &mut playback {
map.insert("muted".into(), serde_json::json!(muted));
map.insert("queue_len".into(), serde_json::json!(queue.total_tracks));
// Overwrite the f32→f64-widened `volume` serde_json::to_value produced
// above with the canonical (3-decimal) form — see `canon_volume`.
map.insert("volume".into(), canon_volume(nominal_volume));
}
let track = queue
@@ -228,7 +231,7 @@ pub fn volume(state: &ApiState, body: &Value) -> Response<Cursor<Vec<u8>>> {
if let Err(e) = state.runtime.core().set_volume(target) {
return runtime_error(&e.to_string());
}
json(200, serde_json::json!({"volume": target, "muted": muted_after}))
json(200, serde_json::json!({"volume": canon_volume(target), "muted": muted_after}))
}
// ============================ internals ============================
@@ -283,7 +286,7 @@ fn apply_mute(state: &ApiState, live: f32, arg: &str) -> Response<Cursor<Vec<u8>
if let Err(e) = set_result {
return runtime_error(&e.to_string());
}
json(200, serde_json::json!({"volume": nominal, "muted": muted_now}))
json(200, serde_json::json!({"volume": canon_volume(nominal), "muted": muted_now}))
}
/// `next`/`previous` (02 §3.3.9-10): gate on NeedsAuth BEFORE running the
@@ -422,4 +425,55 @@ mod tests {
assert_eq!(repeat_str(qbz_models::RepeatMode::All), "all");
assert_eq!(repeat_str(qbz_models::RepeatMode::One), "one");
}
/// A minimal `PlaybackEvent` for the volume-serialization tests below —
/// same shape `player.get_playback_event()` returns, just hand-built so
/// the test does not need a live `ApiState`/runtime.
fn sample_event(volume: f32) -> qbz_player::player::PlaybackEvent {
qbz_player::player::PlaybackEvent {
is_playing: true,
position: 10,
duration: 200,
track_id: 42,
volume,
sample_rate: None,
bit_depth: None,
shuffle: Some(false),
repeat: Some("off".into()),
normalization_gain: None,
gapless_ready: false,
gapless_next_track_id: 0,
bit_perfect_mode: None,
buffer_progress: None,
}
}
#[test]
fn now_playing_map_path_serializes_canonical_volume() {
// Pins the exact `now_playing` sequence: `to_value(&ev)` (which
// widens f32 volume via `Number::from_f32`), then the map overwrite
// that replaces it with `canon_volume`. 0.8f32 must land as `0.8`,
// never `0.800000011920929`.
let ev = sample_event(0.8f32);
let mut playback = serde_json::to_value(&ev).unwrap();
if let Value::Object(map) = &mut playback {
map.insert("muted".into(), serde_json::json!(false));
map.insert("queue_len".into(), serde_json::json!(3));
map.insert("volume".into(), canon_volume(ev.volume));
}
let rendered = serde_json::to_string(&playback).unwrap();
assert!(rendered.contains("\"volume\":0.8"), "got: {rendered}");
assert!(!rendered.contains("0.80000"), "got: {rendered}");
}
#[test]
fn volume_post_response_serializes_canonical_volume() {
// Pins the `/api/playback/volume` and `mute` response builders'
// `json!({"volume": canon_volume(_), ...})` shape.
let target = 0.8f32;
let body = serde_json::json!({"volume": canon_volume(target), "muted": false});
let rendered = serde_json::to_string(&body).unwrap();
assert!(rendered.contains("\"volume\":0.8"), "got: {rendered}");
assert!(!rendered.contains("0.80000"), "got: {rendered}");
}
}
+24 -4
View File
@@ -93,10 +93,14 @@ pub fn info(state: &super::ApiState) -> Response<Cursor<Vec<u8>>> {
/// the CLI maps degradation (needs_auth, missing device) to exit codes.
pub fn status(state: &super::ApiState) -> Response<Cursor<Vec<u8>>> {
let doc = assemble_live(state);
super::json(
200,
serde_json::to_value(&doc).unwrap_or_else(|_| serde_json::json!({})),
)
let mut value = serde_json::to_value(&doc).unwrap_or_else(|_| serde_json::json!({}));
// `StatusDoc.playback.volume` is f32; plain `to_value` widens it via
// `Number::from_f32` (f32→f64, `0.8` → `0.800000011920929`). Overwrite
// with the canonical form — see `super::canon_volume`.
if let Some(vol) = value.pointer_mut("/playback/volume") {
*vol = super::canon_volume(doc.playback.volume);
}
super::json(200, value)
}
/// Compose [`StatusDoc`] from live sources: `DaemonShared` (auth/qconnect/latched
@@ -348,6 +352,22 @@ mod tests {
assert_eq!(json["state"], "needs_auth");
}
#[test]
fn status_doc_playback_volume_serializes_canonically() {
// Pins the `status()` pointer-overwrite: `to_value(&doc)` widens the
// f32 `playback.volume` via `Number::from_f32`; the fix must land
// `0.8` on the wire, never `0.800000011920929`.
let mut doc = needs_auth_doc();
doc.playback.volume = 0.8f32;
let mut value = serde_json::to_value(&doc).unwrap();
if let Some(vol) = value.pointer_mut("/playback/volume") {
*vol = crate::api::canon_volume(doc.playback.volume);
}
let rendered = serde_json::to_string(&value).unwrap();
assert!(rendered.contains("\"volume\":0.8"), "got: {rendered}");
assert!(!rendered.contains("0.80000"), "got: {rendered}");
}
#[test]
fn audio_block_serializes_the_documented_keys() {
// 02 §3.3.3 audio object — the shape the live assembler fills.