polymech/zeroclaw
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(hardware): standardize no device summary string and improve error handling in GPIO tools

Browse Source
This commit is contained in:
ehushubhamshaw 2026-02-26 20:17:35 -05:00
parent ffa9d12432
commit b46efd37f7
9 changed files with 152 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
src/agent/loop_.rs
View File

@ -1167,7 +1167,7 @@ pub(crate) async fn run_tool_call_loop(
/// Append the hardware device summary to the system prompt if non-trivial.
fn append_hw_summary(system_prompt: &mut String, hw_device_summary: &str) {
if !hw_device_summary.is_empty()
&& hw_device_summary != "No hardware devices connected."
&& hw_device_summary != crate::hardware::NO_HW_DEVICES_SUMMARY
{
system_prompt.push_str("\n## Connected Hardware Devices\n\n");
system_prompt.push_str(hw_device_summary);
@ -1879,6 +1879,54 @@ pub async fn process_message(config: Config, message: &str) -> Result<String> {
mod tests {
use super::*;
#[test]
fn parse_shorthand_valid_object_returns_call() {
let result = parse_shorthand_tag_call(r#"shell{"command":"ls -la"}"#);
let call = result.expect("should parse valid shorthand");
assert_eq!(call.name, "shell");
assert_eq!(
call.arguments.get("command").and_then(|v| v.as_str()),
Some("ls -la")
);
}
#[test]
fn parse_shorthand_valid_with_trailing_junk_still_parses() {
// Trailing content after the closing '}' must not prevent parsing.
let result = parse_shorthand_tag_call(r#"my_tool{"key":true} extra junk"#);
let call = result.expect("should parse despite trailing junk");
assert_eq!(call.name, "my_tool");
assert_eq!(call.arguments["key"], serde_json::json!(true));
}
#[test]
fn parse_shorthand_identifier_starts_with_digit_returns_none() {
assert!(parse_shorthand_tag_call(r#"1bad{"k":1}"#).is_none());
}
#[test]
fn parse_shorthand_identifier_with_disallowed_chars_returns_none() {
// Hyphens are not allowed in the identifier.
assert!(parse_shorthand_tag_call(r#"bad-name{"k":1}"#).is_none());
}
#[test]
fn parse_shorthand_array_value_returns_none() {
// JSON after identifier is an array, not an object.
assert!(parse_shorthand_tag_call(r#"tool[1,2,3]"#).is_none());
}
#[test]
fn parse_shorthand_literal_value_returns_none() {
// JSON after identifier is a scalar literal, not an object.
assert!(parse_shorthand_tag_call(r#"tool"string""#).is_none());
}
#[test]
fn parse_shorthand_empty_identifier_returns_none() {
assert!(parse_shorthand_tag_call(r#"{"k":1}"#).is_none());
}
#[test]
fn test_scrub_credentials() {
let input = "API_KEY=sk-1234567890abcdef; token: 1234567890; password=\"secret123456\"";

2
src/channels/mod.rs
View File

@ -1348,7 +1348,7 @@ pub async fn start_channels(config: Config) -> Result<()> {
// Inject hardware device summary if available
if !hw_device_summary.is_empty()
&& hw_device_summary != "No hardware devices connected."
&& hw_device_summary != crate::hardware::NO_HW_DEVICES_SUMMARY
{
system_prompt.push_str("\n## Connected Hardware Devices\n\n");
system_prompt.push_str(&hw_device_summary);

28
src/firmware/pico/main.py
View File

@ -4,6 +4,13 @@
# Wire protocol:
# Host → Device: {"cmd":"gpio_write","params":{"pin":25,"value":1}}\n
# Device → Host: {"ok":true,"data":{"pin":25,"value":1,"state":"HIGH"}}\n
#
# Pin direction policy:
# gpio_write always configures the pin as OUTPUT and caches it.
# gpio_read uses the cached Pin object if one already exists, so a pin
# that was set via gpio_write retains its OUTPUT direction — it is NOT
# reconfigured to INPUT. If no cached Pin exists the pin is opened as
# INPUT and the new Pin is cached for subsequent reads.
import sys
import json
@ -12,6 +19,11 @@ from machine import Pin
# Onboard LED — GPIO 25 on Pico 1
led = Pin(25, Pin.OUT)
# Cache of Pin objects keyed by pin number (excludes the onboard LED on 25).
# gpio_write stores pins as OUTPUT; gpio_read reuses the existing Pin if one
# is cached rather than clobbering its direction.
pins_cache = {}
def handle(msg):
cmd = msg.get("cmd")
params = msg.get("params", {})
@ -40,7 +52,10 @@ def handle(msg):
if pin_num == 25:
led.value(value)
else:
Pin(pin_num, Pin.OUT).value(value)
# Always (re-)configure as OUT so subsequent reads on this pin
# reflect the driven state rather than clobbering the direction.
pins_cache[pin_num] = Pin(pin_num, Pin.OUT)
pins_cache[pin_num].value(value)
state = "HIGH" if value == 1 else "LOW"
return {"ok": True, "data": {"pin": pin_num, "value": value, "state": state}}
@ -55,7 +70,10 @@ def handle(msg):
return {"ok": False, "error": "invalid pin"}
if pin_num < 0:
return {"ok": False, "error": "invalid pin"}
value = led.value() if pin_num == 25 else Pin(pin_num, Pin.IN).value()
value = led.value() if pin_num == 25 else (
pins_cache[pin_num].value() if pin_num in pins_cache
else pins_cache.setdefault(pin_num, Pin(pin_num, Pin.IN)).value()
)
state = "HIGH" if value == 1 else "LOW"
return {"ok": True, "data": {"pin": pin_num, "value": value, "state": state}}
@ -70,5 +88,9 @@ while True:
msg = json.loads(line)
result = handle(msg)
print(json.dumps(result))
except Exception as e:
except (ValueError, KeyError, TypeError) as e:
# ValueError — json.loads() on malformed input
# KeyError — unexpected missing key in a message dict
# TypeError — wrong type in an operation
# Any other exception propagates so bugs surface during development.
print(json.dumps({"ok": False, "error": str(e)}))

20
src/hardware/device.rs
View File

@ -164,6 +164,11 @@ struct RegisteredDevice {
capabilities: DeviceCapabilities,
}
/// Summary string returned by [`DeviceRegistry::prompt_summary`] when no
/// devices are registered. Exported so callers can compare against it without
/// duplicating the literal.
pub const NO_HW_DEVICES_SUMMARY: &str = "No hardware devices connected.";
/// Registry of discovered devices with stable session aliases.
///
/// - Scans at startup (via `hardware::discover`)
@ -230,15 +235,21 @@ impl DeviceRegistry {
}
/// Attach a transport and capabilities to a previously registered device.
///
/// Returns `Err` when `alias` is not found in the registry (should not
/// happen in normal usage because callers pass aliases from `register`).
pub fn attach_transport(
&mut self,
alias: &str,
transport: Arc<dyn Transport>,
capabilities: DeviceCapabilities,
) {
) -> anyhow::Result<()> {
if let Some(entry) = self.devices.get_mut(alias) {
entry.transport = Some(transport);
entry.capabilities = capabilities;
Ok(())
} else {
Err(anyhow::anyhow!("unknown device alias: {}", alias))
}
}
@ -268,7 +279,7 @@ impl DeviceRegistry {
/// Return a summary of connected devices for the LLM system prompt.
pub fn prompt_summary(&self) -> String {
if self.devices.is_empty() {
return "No hardware devices connected.".to_string();
return NO_HW_DEVICES_SUMMARY.to_string();
}
let mut lines = vec!["Connected devices:".to_string()];
@ -467,7 +478,8 @@ impl DeviceRegistry {
gpio: true, // assume GPIO; Phase 3 will populate via capabilities handshake
..DeviceCapabilities::default()
};
registry.attach_transport(&alias, transport, caps);
registry.attach_transport(&alias, transport, caps)
.unwrap_or_else(|e| tracing::warn!(alias = %alias, err = %e, "attach_transport: unexpected unknown alias"));
tracing::info!(
alias = %alias,
@ -648,7 +660,7 @@ mod tests {
#[test]
fn registry_prompt_summary_empty() {
let reg = DeviceRegistry::new();
assert_eq!(reg.prompt_summary(), "No hardware devices connected.");
assert_eq!(reg.prompt_summary(), NO_HW_DEVICES_SUMMARY);
}
#[test]

3
src/hardware/gpio.rs
View File

@ -339,7 +339,8 @@ mod tests {
gpio: true,
..Default::default()
},
);
)
.expect("alias was just registered");
Arc::new(RwLock::new(reg))
}

5
src/hardware/mod.rs
View File

@ -33,7 +33,7 @@ pub mod manifest;
pub mod subprocess;
pub mod tool_registry;
pub use device::{Device, DeviceCapabilities, DeviceContext, DeviceKind, DeviceRegistry, DeviceRuntime};
pub use device::{Device, DeviceCapabilities, DeviceContext, DeviceKind, DeviceRegistry, DeviceRuntime, NO_HW_DEVICES_SUMMARY};
pub use gpio::{gpio_tools, GpioReadTool, GpioWriteTool};
#[cfg(feature = "hardware")]
pub use pico_code::{device_code_tools, DeviceExecTool, DeviceReadCodeTool, DeviceWriteCodeTool};
@ -135,7 +135,8 @@ pub async fn boot(peripherals: &crate::config::PeripheralsConfig) -> anyhow::Res
HardwareSerialTransport::new(&path, board.baud),
) as std::sync::Arc<dyn transport::Transport>;
let caps = DeviceCapabilities { gpio: true, ..DeviceCapabilities::default() };
registry_inner.attach_transport(&alias, transport, caps);
registry_inner.attach_transport(&alias, transport, caps)
.unwrap_or_else(|e| tracing::warn!(alias = %alias, err = %e, "attach_transport: unexpected unknown alias"));
// Mark path as registered so duplicate config entries are skipped.
discovered_paths.insert(path.clone());
tracing::info!(

78
src/hardware/subprocess.rs
View File

@ -177,47 +177,57 @@ impl Tool for SubprocessTool {
}
};
// Kill child, wait for exit status, and collect stderr — done before
// the match so all error branches can include them.
let _ = child.kill().await;
let child_status = child.wait().await.ok();
let stderr_msg = collect_stderr(stderr_handle).await;
match read_result {
// ── Timeout ────────────────────────────────────────────────────
Err(_elapsed) => Ok(ToolResult {
success: false,
output: String::new(),
error: Some(format!(
"plugin '{}' timed out after {}s{}",
self.manifest.tool.name,
SUBPROCESS_TIMEOUT_SECS,
if stderr_msg.is_empty() {
String::new()
} else {
format!("; stderr: {}", stderr_msg)
}
)),
}),
// The read deadline elapsed — force-kill the plugin and collect
// any stderr it emitted before dying.
Err(_elapsed) => {
let _ = child.kill().await;
let _ = child.wait().await;
let stderr_msg = collect_stderr(stderr_handle).await;
Ok(ToolResult {
success: false,
output: String::new(),
error: Some(format!(
"plugin '{}' timed out after {}s{}",
self.manifest.tool.name,
SUBPROCESS_TIMEOUT_SECS,
if stderr_msg.is_empty() {
String::new()
} else {
format!("; stderr: {}", stderr_msg)
}
)),
})
}
// ── I/O error reading stdout ───────────────────────────────────
Ok(Err(io_err)) => Ok(ToolResult {
success: false,
output: String::new(),
error: Some(format!(
"plugin '{}': I/O error reading stdout: {}{}",
self.manifest.tool.name,
io_err,
if stderr_msg.is_empty() {
String::new()
} else {
format!("; stderr: {}", stderr_msg)
}
)),
}),
Ok(Err(io_err)) => {
let _ = child.kill().await;
let _ = child.wait().await;
let stderr_msg = collect_stderr(stderr_handle).await;
Ok(ToolResult {
success: false,
output: String::new(),
error: Some(format!(
"plugin '{}': I/O error reading stdout: {}{}",
self.manifest.tool.name,
io_err,
if stderr_msg.is_empty() {
String::new()
} else {
format!("; stderr: {}", stderr_msg)
}
)),
})
}
// ── Got a line ────────────────────────────────────────────────
// Let the process finish naturally — plugins that write their
// result and then do cleanup should not be interrupted.
Ok(Ok(line)) => {
let child_status = child.wait().await.ok();
let stderr_msg = collect_stderr(stderr_handle).await;
let line = line.trim();
if line.is_empty() {

12
src/peripherals/serial.rs
View File

@ -285,10 +285,14 @@ impl Tool for GpioWriteTool {
.and_then(|v| v.as_u64())
.ok_or_else(|| anyhow::anyhow!("Missing 'value' parameter"))?;
if value != 0 && value != 1 {
return Err(anyhow::anyhow!(
"Invalid 'value' parameter: expected 0 or 1, got {}",
value
));
return Ok(ToolResult {
success: false,
output: String::new(),
error: Some(format!(
"Invalid 'value' parameter: expected 0 or 1, got {}",
value
)),
});
}
self.transport
.request("gpio_write", json!({ "pin": pin, "value": value }))

7
src/util.rs
View File

@ -204,8 +204,9 @@ pub fn is_serial_path_allowed(path: &str) -> bool {
}
// ── Fallback (other platforms) ────────────────────────────────────────
// Reject unknown platforms rather than applying a too-permissive prefix
// match. If support is needed for a new platform, add a targeted branch
// above.
#[allow(unreachable_code)]
ALLOWED_SERIAL_PATH_PREFIXES
.iter()
.any(|p| path.starts_with(p))
false
}