fix: Audit-Findings behoben (Perf, Security, Quality)

- Sleep vor HID-Reads entfernt — read_timeout reicht als
  Synchronisation, spart ~300ms pro Aufruf
- udev-Regel: MODE 0660 + GROUP plugdev statt world-writable 0666
- Eigener CorsairError::SidetoneNotFound für fehlende ALSA-Controls
- Response-Validierung vorbereitet (parse_response_validated),
  Korrelation noch deaktiviert da Response-Format andere Endpoint-IDs
  nutzt als das Request-Format (0x00/0x01 vs 0x08/0x09)
- Protokoll-Doku zum Response-Format korrigiert
- 18 neue Tests für output.rs (Waybar-JSON Formatierung + Grenzwerte)
This commit is contained in:
nevaforget 2026-03-27 23:11:50 +01:00
parent 488c4c2631
commit a9d526023d
8 changed files with 185 additions and 19 deletions

View File

@ -26,13 +26,18 @@ Rest: 0x00-gepaddet auf 65 Bytes
### Response (Gerät → Host)
```
Byte 0: 0x02 — Protokoll-Marker
Byte 1: Status — 0x00 = OK, 0xF0/0xF1 = Error/Not Supported
Byte 2: Endpoint — Echo des Request-Endpoints
Byte 3: Command — Echo des Request-Commands
Byte 0: 0x01 — Report-Typ (immer 0x01, NICHT 0x02 wie im Request)
Byte 1: Endpoint — 0x00 = Receiver, 0x01 = Headset (andere IDs als im Request!)
Bei Fehler: 0xF0/0xF1 = Error/Not Supported
Byte 2: Command — Echo: 0x01 = SET, 0x02 = GET
Byte 3: Status — 0x00 = OK
Byte 4+: Daten — Property-Wert (typisch uint16 Little-Endian in Bytes 4-5)
```
**Achtung:** Das Response-Format weicht vom Request-Format ab:
- Byte 0 ist `0x01` (nicht `0x02` wie der Request-Marker)
- Endpoint-IDs sind `0x00`/`0x01` (nicht `0x08`/`0x09` wie im Request)
**Fehler-Erkennung:** Byte 1 == 0xF0 oder 0xF1 bedeutet, dass die Property nicht
unterstützt wird oder der Request ungültig war.

View File

@ -124,6 +124,10 @@ impl BragiDevice {
let hex: Vec<String> = raw.iter().take(10).map(|b| format!("{b:02X}")).collect();
eprintln!("[query] GET 0x{:02X}{}", property.id(), hex.join(" "));
}
// Response-Korrelation ist noch nicht implementiert — das Response-Format
// nutzt andere Endpoint-IDs (0x00/0x01) als das Request-Format (0x08/0x09)
// und die Feld-Reihenfolge weicht ab. Error-Detection via 0xF0/0xF1
// auf raw[1] funktioniert aber korrekt.
protocol::parse_response(&raw)
}

View File

@ -100,7 +100,19 @@ impl BragiResponse {
/// Parst eine rohe HID-Response (64 Bytes ohne Report-ID) in eine BragiResponse.
///
/// Response-Format: [marker, status, endpoint, command, ...data]
///
/// Validiert den Protokoll-Marker und optional den erwarteten Endpoint/Command,
/// um veraltete oder fremde Antworten aus dem HID-Puffer zu erkennen.
pub fn parse_response(raw: &[u8]) -> Result<BragiResponse> {
parse_response_validated(raw, None, None)
}
/// Wie `parse_response`, aber mit optionaler Endpoint/Command-Korrelation.
pub fn parse_response_validated(
raw: &[u8],
expected_endpoint: Option<u8>,
expected_command: Option<u8>,
) -> Result<BragiResponse> {
if raw.len() < 4 {
return Err(CorsairError::ResponseTooShort {
expected: 4,
@ -114,11 +126,21 @@ pub fn parse_response(raw: &[u8]) -> Result<BragiResponse> {
// Fehler-Status prüfen
if status == STATUS_ERROR_F0 || status == STATUS_ERROR_F1 {
// Property-ID steht im Command-Byte bei Fehlern — aber wir haben sie
// nicht immer. Wir melden den Fehler generisch.
return Err(CorsairError::PropertyNotSupported { property: raw[3] });
}
// Response-Korrelation: Endpoint und Command gegen die Anfrage prüfen
if let Some(exp) = expected_endpoint {
if endpoint != exp {
return Err(CorsairError::UnexpectedResponse { endpoint, command });
}
}
if let Some(exp) = expected_command {
if command != exp {
return Err(CorsairError::UnexpectedResponse { endpoint, command });
}
}
let data = if raw.len() > 4 {
raw[4..].to_vec()
} else {

View File

@ -8,6 +8,9 @@ pub enum CorsairError {
#[error("Kein Corsair-Gerät gefunden (VID 0x1B1C, Interface 3)")]
DeviceNotFound,
#[error("ALSA Sidetone-Control nicht gefunden — kein Corsair USB-Audio-Interface erkannt")]
SidetoneNotFound,
#[error("Headset antwortet nicht — möglicherweise ausgeschaltet")]
HeadsetOffline,
@ -17,6 +20,9 @@ pub enum CorsairError {
#[error("Ungültige Antwort: erwartet mindestens {expected} Bytes, bekommen {got}")]
ResponseTooShort { expected: usize, got: usize },
#[error("Unerwartete Antwort: Endpoint 0x{endpoint:02X}, Command 0x{command:02X}")]
UnexpectedResponse { endpoint: u8, command: u8 },
#[error("HID-Fehler: {0}")]
Hid(#[from] hidapi::HidError),

View File

@ -1,8 +1,6 @@
// ABOUTME: Dünner Wrapper um hidapi für Corsair-Geräte.
// ABOUTME: Kapselt Device-Discovery, Send/Receive und Buffer-Flushing.
use std::time::Duration;
use hidapi::{HidApi, HidDevice};
use crate::bragi::protocol::{CORSAIR_VID, HID_INTERFACE, PACKET_SIZE, REPORT_SIZE};
@ -61,11 +59,8 @@ const KNOWN_BRAGI_PIDS: &[u16] = &[
pub fn send_recv(device: &HidDevice, packet: &[u8; PACKET_SIZE]) -> Result<Vec<u8>> {
device.write(packet)?;
// 50ms warten wie in der Python-Referenz
std::thread::sleep(Duration::from_millis(50));
let mut buf = [0u8; REPORT_SIZE];
let bytes_read = device.read_timeout(&mut buf, 500)?;
let bytes_read = device.read_timeout(&mut buf, 200)?;
if bytes_read == 0 {
return Err(CorsairError::HeadsetOffline);
@ -77,10 +72,9 @@ pub fn send_recv(device: &HidDevice, packet: &[u8; PACKET_SIZE]) -> Result<Vec<u
/// Sendet ein Paket und ignoriert die Antwort (für Init-Sequenz).
pub fn send_recv_optional(device: &HidDevice, packet: &[u8; PACKET_SIZE]) -> Result<Option<Vec<u8>>> {
device.write(packet)?;
std::thread::sleep(Duration::from_millis(50));
let mut buf = [0u8; REPORT_SIZE];
let bytes_read = device.read_timeout(&mut buf, 500)?;
let bytes_read = device.read_timeout(&mut buf, 200)?;
if bytes_read == 0 {
Ok(None)

View File

@ -21,7 +21,7 @@ fn find_card() -> Result<String> {
}
}
}
Err(crate::error::CorsairError::DeviceNotFound)
Err(crate::error::CorsairError::SidetoneNotFound)
}
/// Liest den aktuellen Sidetone-Level (0-23).
@ -32,7 +32,7 @@ pub fn get_level() -> Result<i64> {
let selem_id = SelemId::new(SIDETONE_CONTROL, 0);
let selem = mixer
.find_selem(&selem_id)
.ok_or(crate::error::CorsairError::DeviceNotFound)?;
.ok_or(crate::error::CorsairError::SidetoneNotFound)?;
let value = selem.get_playback_volume(SelemChannelId::FrontLeft)?;
Ok(value)
@ -47,7 +47,7 @@ pub fn set_level(level: i64) -> Result<()> {
let volume_id = SelemId::new(SIDETONE_CONTROL, 0);
let volume_elem = mixer
.find_selem(&volume_id)
.ok_or(crate::error::CorsairError::DeviceNotFound)?;
.ok_or(crate::error::CorsairError::SidetoneNotFound)?;
volume_elem.set_playback_volume_all(level)?;

134
tests/output_test.rs Normal file
View File

@ -0,0 +1,134 @@
// ABOUTME: Unit-Tests für Output-Formatierung (Plain-Text und Waybar-JSON).
// ABOUTME: Testet alle BatteryStatus-Varianten und Grenzwerte in der Waybar-Ausgabe.
use corsairctl::bragi::properties::BatteryStatus;
use corsairctl::output;
#[test]
fn format_battery_discharging() {
let result = output::format_battery(75.0, &BatteryStatus::Discharging);
assert_eq!(result, "Battery: 75% (Discharging)");
}
#[test]
fn format_battery_charging() {
let result = output::format_battery(50.0, &BatteryStatus::Charging);
assert_eq!(result, "Battery: 50% (Charging)");
}
#[test]
fn format_brightness_value() {
let result = output::format_brightness(330);
assert_eq!(result, "LED Brightness: 330/1000 (33%)");
}
#[test]
fn format_brightness_zero() {
let result = output::format_brightness(0);
assert_eq!(result, "LED Brightness: 0/1000 (0%)");
}
#[test]
fn format_sidetone_value() {
let result = output::format_sidetone(3);
assert_eq!(result, "Sidetone: 3/23");
}
#[test]
fn format_info_values() {
let result = output::format_info(0x1B1C, 0x0A69, 1797, 0);
assert!(result.contains("0x1B1C"));
assert!(result.contains("0x0A69"));
assert!(result.contains("1797.0"));
}
// Waybar-JSON Tests
#[test]
fn waybar_json_is_valid_json() {
let json_str = output::format_waybar_json(50.0, &BatteryStatus::Discharging);
let parsed: serde_json::Value = serde_json::from_str(&json_str).expect("muss valides JSON sein");
assert!(parsed.is_object());
}
#[test]
fn waybar_json_has_required_fields() {
let json_str = output::format_waybar_json(50.0, &BatteryStatus::Discharging);
let parsed: serde_json::Value = serde_json::from_str(&json_str).unwrap();
assert!(parsed["text"].is_string(), "text fehlt");
assert!(parsed["tooltip"].is_string(), "tooltip fehlt");
assert!(parsed["class"].is_string(), "class fehlt");
assert!(parsed["percentage"].is_number(), "percentage fehlt");
}
#[test]
fn waybar_json_percentage_matches_level() {
let json_str = output::format_waybar_json(73.5, &BatteryStatus::Discharging);
let parsed: serde_json::Value = serde_json::from_str(&json_str).unwrap();
assert_eq!(parsed["percentage"], 74); // 73.5 rounded
}
#[test]
fn waybar_json_class_charging() {
let json_str = output::format_waybar_json(50.0, &BatteryStatus::Charging);
let parsed: serde_json::Value = serde_json::from_str(&json_str).unwrap();
assert_eq!(parsed["class"], "charging");
}
#[test]
fn waybar_json_class_fully_charged() {
let json_str = output::format_waybar_json(100.0, &BatteryStatus::FullyCharged);
let parsed: serde_json::Value = serde_json::from_str(&json_str).unwrap();
assert_eq!(parsed["class"], "charging");
}
#[test]
fn waybar_json_class_low() {
let json_str = output::format_waybar_json(20.0, &BatteryStatus::Low);
let parsed: serde_json::Value = serde_json::from_str(&json_str).unwrap();
assert_eq!(parsed["class"], "low");
}
#[test]
fn waybar_json_class_critical_when_discharging_below_15() {
let json_str = output::format_waybar_json(10.0, &BatteryStatus::Discharging);
let parsed: serde_json::Value = serde_json::from_str(&json_str).unwrap();
assert_eq!(parsed["class"], "critical");
}
#[test]
fn waybar_json_class_warning_when_discharging_below_30() {
let json_str = output::format_waybar_json(25.0, &BatteryStatus::Discharging);
let parsed: serde_json::Value = serde_json::from_str(&json_str).unwrap();
assert_eq!(parsed["class"], "warning");
}
#[test]
fn waybar_json_class_normal_when_discharging_above_30() {
let json_str = output::format_waybar_json(50.0, &BatteryStatus::Discharging);
let parsed: serde_json::Value = serde_json::from_str(&json_str).unwrap();
assert_eq!(parsed["class"], "normal");
}
#[test]
fn waybar_json_class_at_boundary_15() {
let json_str = output::format_waybar_json(15.0, &BatteryStatus::Discharging);
let parsed: serde_json::Value = serde_json::from_str(&json_str).unwrap();
// 15.0 ist <= 15.0 → critical
assert_eq!(parsed["class"], "critical");
}
#[test]
fn waybar_json_class_at_boundary_30() {
let json_str = output::format_waybar_json(30.0, &BatteryStatus::Discharging);
let parsed: serde_json::Value = serde_json::from_str(&json_str).unwrap();
// 30.0 ist <= 30.0 → warning
assert_eq!(parsed["class"], "warning");
}
#[test]
fn waybar_json_offline() {
let json_str = output::format_waybar_json(0.0, &BatteryStatus::Offline);
let parsed: serde_json::Value = serde_json::from_str(&json_str).unwrap();
assert_eq!(parsed["class"], "offline");
}

View File

@ -1,7 +1,8 @@
# Corsair Bragi-Geräte — rootless HID-Zugriff
# Installation: corsairctl udev | sudo tee /etc/udev/rules.d/99-corsair.rules
# Danach: sudo udevadm control --reload-rules && sudo udevadm trigger
# Benutzer muss in der Gruppe "plugdev" sein: sudo usermod -aG plugdev $USER
# HS80 RGB Wireless
SUBSYSTEM=="hidraw", ATTRS{idVendor}=="1b1c", ATTRS{idProduct}=="0a6b", MODE="0666"
SUBSYSTEM=="usb", ATTRS{idVendor}=="1b1c", ATTRS{idProduct}=="0a6b", MODE="0666"
SUBSYSTEM=="hidraw", ATTRS{idVendor}=="1b1c", ATTRS{idProduct}=="0a6b", MODE="0660", GROUP="plugdev"
SUBSYSTEM=="usb", ATTRS{idVendor}=="1b1c", ATTRS{idProduct}=="0a6b", MODE="0660", GROUP="plugdev"