blueflare/bfe-core1106-sdk
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

review: iteration-1 fixes across CI, bridge, VM harness, and docs

Browse Source 
CI/pipeline:
- KERNEL_TARBALL passed as a YAML env literal '~' was never tilde-expanded
  and would have failed every hosted kernel-build dispatch; the path is now
  exported from the shell. Verified reproducible before the fix.
- Every job gets timeout-minutes; boot smoke uses timeout -k so a wedged
  qemu is SIGKILLed instead of holding the job.
- Tarball fetch + fail-closed sha256 verification deduplicated into
  build/fetch-kernel-tarball.sh (with curl retries), used by build-kernel.sh
  and both CI jobs. busybox fetch gains retries too.
- ccache layer for kernel-build (cache keyed on defconfig+patches) recovers
  the incremental-compile speed the ephemeral-runner move cost.
- build-kernel.sh now asserts every fragment option survived olddefconfig —
  merge_config -m pastes text and Kconfig silently drops unmet symbols.

rs485-bridge:
- pending-buffer cap (2x max RTU ADU) instead of unbounded growth;
  explicit accept-loop error handling with backoff instead of .flatten();
  per-arm inline bounds instead of the string-keyed lookup whose default
  would have mis-bounded a future get-input; control-socket cleanup errors
  surfaced; flag-shaped values rejected in arg parsing; doc example uses a
  private mktemp dir. Test timing margins widened for contended runners
  (gap 25->120ms, 60x margin on the split-frame test).

VM harness:
- stage-1/stage-2 boot scripts share one validated slot parser and one
  by-name populator (qemu/rootfs/etc/warden-lib.sh) — the duplicated
  parser had already diverged on validation; userdata/oem mount failures
  now fail fast with a greppable sentinel; udhcpc fallback keys off the
  interface actually having an address; switch_root applet guarded.
- boot-smoke delegates the qemu invocation to run.sh (machine shape lives
  in ONE place); run.sh port 0 disables a hostfwd.
- mkimage: unknown partition names fail at build time; DISK_END is a max,
  not last-entry; --state keys validated as filenames.
- portal-scenario: mock readiness is asserted (no silent fall-through),
  hostfwd port collisions retried, mount-failure sentinel fails fast.
- ui-shot: fixed sleeps replaced with bounded screendump polling; the
  repaint assertion is real and documented as such. qmp.py loses its
  module-global and gains argv validation.

Docs/scrub: bench-host paths and the site AP name removed from six more
port docs and two evidence tables; path-bearing build artifacts (.elf,
.map) untracked (the 154-byte firmware .bin is path-free and stays);
ADR-0003 marked visibility-superseded by ADR-0007; stale section
cross-reference fixed; flare-edge noted as private for outside readers;
stale root-level review report removed per the new workspace rule.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018HUayid7W5w7jBdb9Rrj1K
This commit is contained in:
BFE Engineering
2026-08-30 08:19:17 -06:00
co-authored by Claude Fable 5
parent b667ff5b1e
commit 2756de0b46
24 changed files with 481 additions and 203 deletions
Download Patch File Download Diff File Expand all files Collapse all files
qemu/rs485-bridge/src/lib.rs
+83 -36
View File
@@ -24,6 +24,11 @@ use warden_sim::ModbusSlave;
/// response timeout is orders of magnitude larger.
pub const DEFAULT_GAP: Duration = Duration::from_millis(10);
/// Accumulation cap: a Modbus RTU ADU is at most 256 bytes, so anything past
/// 2x that without an inter-frame gap is a misbehaving master streaming
/// continuously — drop the buffer instead of growing without bound.
const MAX_PENDING: usize = 512;
/// The shared bus: the slave plus its declared dimensions. The sim's register
/// setters panic on out-of-range indices (deliberate test-harness semantics);
/// the control channel must bounds-check first so a typo in a scenario script
@@ -64,7 +69,17 @@ pub fn pump_serial(
}
return Ok(());
}
Ok(n) => buf.extend_from_slice(&chunk[..n]),
Ok(n) => {
buf.extend_from_slice(&chunk[..n]);
if buf.len() > MAX_PENDING {
eprintln!(
"rs485: {} bytes buffered with no inter-frame gap — discarding \
(misbehaving master streaming continuously?)",
buf.len()
);
buf.clear();
}
}
Err(e)
if e.kind() == std::io::ErrorKind::WouldBlock
|| e.kind() == std::io::ErrorKind::TimedOut =>
@@ -127,10 +142,10 @@ pub fn handle_control_line(line: &str, bus: &Bus) -> String {
if words.next().is_some() {
return format!("err trailing arguments after '{cmd}'");
}
let bound = |cmd: &str| match cmd {
"holding" | "input" | "get-holding" => bus.regs,
_ => bus.bits,
};
// Each arm states its own bound (bus.regs for register space, bus.bits for
// bit space) INLINE — a previous string-keyed lookup defaulted silently to
// the bit bound, which would have handed a future `get-input` command the
// wrong range and reintroduced the out-of-range panic this check prevents.
let mut s = bus.slave.lock().unwrap();
match (cmd, arg) {
("ping", None) => "ok".into(),
@@ -152,39 +167,63 @@ pub fn handle_control_line(line: &str, bus: &Bus) -> String {
}
_ => format!("err bad exception code '{c}'"),
},
("holding" | "input" | "coil" | "discrete", Some(kv)) => {
let (addr, val) = match kv.split_once('=') {
Some((a, v)) => (parse_u16(a), parse_u16(v)),
None => (None, None),
};
match (addr, val) {
(Some(a), _) if (a as usize) >= bound(cmd) => {
format!("err address {a} out of range (0..{})", bound(cmd))
("holding" | "input", Some(kv)) => match parse_addr_val(kv, bus.regs) {
Ok((a, v)) => {
if cmd == "holding" {
s.set_holding(a, v);
} else {
s.set_input(a, v);
}
(Some(a), Some(v)) => {
match cmd {
"holding" => s.set_holding(a as usize, v),
"input" => s.set_input(a as usize, v),
"coil" => s.set_coil(a as usize, v != 0),
_ => s.set_discrete(a as usize, v != 0),
}
"ok".into()
"ok".into()
}
Err(e) => e,
},
("coil" | "discrete", Some(kv)) => match parse_addr_val(kv, bus.bits) {
Ok((a, v)) => {
if cmd == "coil" {
s.set_coil(a, v != 0);
} else {
s.set_discrete(a, v != 0);
}
_ => format!("err expected <addr>=<value>, got '{kv}'"),
"ok".into()
}
}
("get-holding" | "get-coil", Some(a)) => match parse_u16(a) {
Some(a) if (a as usize) >= bound(cmd) => {
format!("err address {a} out of range (0..{})", bound(cmd))
}
Some(a) if cmd == "get-holding" => format!("ok {}", s.holding(a as usize)),
Some(a) => format!("ok {}", u8::from(s.coil(a as usize))),
None => format!("err bad address '{a}'"),
Err(e) => e,
},
("get-holding", Some(a)) => match parse_addr(a, bus.regs) {
Ok(a) => format!("ok {}", s.holding(a)),
Err(e) => e,
},
("get-coil", Some(a)) => match parse_addr(a, bus.bits) {
Ok(a) => format!("ok {}", u8::from(s.coil(a))),
Err(e) => e,
},
_ => format!("err unknown or malformed command '{line}'"),
}
}
/// Parse "<addr>=<value>" with the address bounds-checked against `bound`.
fn parse_addr_val(kv: &str, bound: usize) -> Result<(usize, u16), String> {
let Some((a, v)) = kv.split_once('=') else {
return Err(format!("err expected <addr>=<value>, got '{kv}'"));
};
let (Some(a), Some(v)) = (parse_u16(a), parse_u16(v)) else {
return Err(format!("err expected <addr>=<value>, got '{kv}'"));
};
if (a as usize) >= bound {
return Err(format!("err address {a} out of range (0..{bound})"));
}
Ok((a as usize, v))
}
/// Parse a bare address, bounds-checked against `bound`.
fn parse_addr(a: &str, bound: usize) -> Result<usize, String> {
match parse_u16(a) {
Some(v) if (v as usize) < bound => Ok(v as usize),
Some(v) => Err(format!("err address {v} out of range (0..{bound})")),
None => Err(format!("err bad address '{a}'")),
}
}
fn parse_u16(s: &str) -> Option<u16> {
if let Some(h) = s.strip_prefix("0x").or_else(|| s.strip_prefix("0X")) {
u16::from_str_radix(h, 16).ok()
@@ -200,10 +239,13 @@ mod tests {
use std::time::Duration;
use warden_sim::modbus::{crc_ok, read_holding};
// Test gap is larger than DEFAULT_GAP so a loaded CI runner cannot split
// a frame that the test wrote in two deliberate chunks.
const GAP: Duration = Duration::from_millis(25);
const SETTLE: Duration = Duration::from_millis(100);
// Test gap is much larger than DEFAULT_GAP so a loaded CI runner cannot
// split a frame the test wrote in two deliberate chunks: the 2ms
// inter-chunk pause has a 60x margin against the 120ms dispatch gap
// (25ms gave only 12.5x and was flagged as a flake risk on contended
// 2-vCPU hosted runners).
const GAP: Duration = Duration::from_millis(120);
const SETTLE: Duration = Duration::from_millis(400);
fn bus() -> Bus {
let b = Bus::new(1, 16, 16);
@@ -221,7 +263,9 @@ mod tests {
}
fn read_reply(master: &UnixStream) -> Vec<u8> {
master.set_read_timeout(Some(Duration::from_secs(2))).unwrap();
master
.set_read_timeout(Some(Duration::from_secs(2)))
.unwrap();
let mut buf = [0u8; 256];
let n = (&*master).read(&mut buf).expect("expected a reply frame");
buf[..n].to_vec()
@@ -274,7 +318,10 @@ mod tests {
with_pump(&s, |master| {
(&*master).write_all(&read_holding(1, 2, 1)).unwrap();
master
.set_read_timeout(Some(Duration::from_millis(200)))
// Well past GAP: the dropped frame must have been dispatched
// (and answered with silence) before the next request is
// written, or the two would merge in the pending buffer.
.set_read_timeout(Some(Duration::from_millis(500)))
.unwrap();
let mut buf = [0u8; 16];
assert!(
qemu/rs485-bridge/src/main.rs
+40 -9
View File
@@ -2,10 +2,14 @@
//! there); this file only parses arguments, connects sockets, and spawns the
//! control listener.
//!
//! Typical use (matches qemu/run.sh --rs485):
//! Typical use (matches qemu/run.sh --rs485). Put the sockets in a private
//! per-run directory (mktemp -d) — short (AF_UNIX caps paths at ~108 chars)
//! and not guessable/pre-creatable by other local users, unlike a fixed
//! /tmp name:
//!
//! qemu/run.sh --kernel ... --rs485 /tmp/warden-rs485.sock &
//! rs485-bridge --serial /tmp/warden-rs485.sock --control /tmp/warden-rs485-ctl.sock
//! d=$(mktemp -d /tmp/rs485.XXXXXX)
//! qemu/run.sh --kernel ... --rs485 "$d/serial.sock" &
//! rs485-bridge --serial "$d/serial.sock" --control "$d/ctl.sock"
use std::io::{BufRead, BufReader, Write};
use std::os::unix::net::{UnixListener, UnixStream};
@@ -31,10 +35,19 @@ fn main() {
let mut args = std::env::args().skip(1);
while let Some(a) = args.next() {
let mut val = |name: &str| args.next().unwrap_or_else(|| {
eprintln!("{name} needs a value");
usage()
});
let mut val = |name: &str| {
let v = args.next().unwrap_or_else(|| {
eprintln!("{name} needs a value");
usage()
});
// A following flag means the value was omitted — report the real
// problem instead of swallowing the flag as a bogus value.
if v.starts_with("--") {
eprintln!("{name} needs a value, got flag '{v}'");
usage()
}
v
};
match a.as_str() {
"--serial" => serial = Some(val("--serial")),
"--control" => control = Some(val("--control")),
@@ -55,14 +68,32 @@ fn main() {
let bus: &'static Bus = Box::leak(Box::new(Bus::new(address, regs, bits)));
if let Some(path) = control {
let _ = std::fs::remove_file(&path); // stale socket from a previous run
// Clear a stale socket from a previous run. A failure here that is not
// "nothing to remove" (e.g. someone else's file behind /tmp's sticky
// bit) will make the bind below fail — surface both errors.
let removed = std::fs::remove_file(&path);
let listener = UnixListener::bind(&path).unwrap_or_else(|e| {
eprintln!("FATAL: cannot bind control socket {path}: {e}");
if let Err(re) = removed {
if re.kind() != std::io::ErrorKind::NotFound {
eprintln!(" (removing the pre-existing file also failed: {re})");
}
}
exit(1);
});
eprintln!("rs485: control socket at {path}");
std::thread::spawn(move || {
for conn in listener.incoming().flatten() {
// Explicit error handling: `.flatten()` would turn a persistent
// accept() failure (fd exhaustion etc.) into a silent hot loop.
for conn in listener.incoming() {
let conn = match conn {
Ok(c) => c,
Err(e) => {
eprintln!("rs485: control accept failed: {e} — backing off");
std::thread::sleep(Duration::from_millis(200));
continue;
}
};
let reader = BufReader::new(conn.try_clone().expect("clone control conn"));
let mut writer = conn;
for line in reader.lines() {