Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
141 changes: 74 additions & 67 deletions crates/stackchan-firmware/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1571,76 +1571,83 @@ async fn main(spawner: Spawner) -> ! {
);
}

// BLE peripheral. Shares the same `&'static esp_radio::Controller`
// as Wi-Fi — coex (enabled via the `coex` feature on `esp-radio`)
// schedules airtime between the two stacks. The BLE address +
// local name are derived from the chip's eFuse MAC so a single
// device shows up consistently across reboots, and matches the
// discovery handle visible on the LAN over mDNS.
#[allow(clippy::items_after_statements)]
static BLE_NAME: StaticCell<heapless::String<32>> = StaticCell::new();
let ble_mac = esp_hal::efuse::Efuse::mac_address();
// First, try the operator-supplied name from
// `/sd/DEVICE.NAM` (set via the Hardware Buddy `cmd:name`
// message). Fall back to the MAC-suffixed default. The name
// must start with `Claude` so the Hardware Buddy picker filters
// us in; the default does, and a desktop-set override is
// assumed to as well — we don't enforce the prefix.
let persisted_name = stackchan_firmware::storage::with_storage(
stackchan_firmware::storage::FirmwareStorage::read_device_name,
)
.await
.and_then(|r| match r {
Ok(opt) => opt,
Err(e) => {
defmt::warn!(
"ble: device name read failed ({}); using MAC-suffixed default",
defmt::Debug2Format(&e)
);
None
}
});
let ble_name: &'static str = {
use core::fmt::Write as _;
let mut s: heapless::String<32> = heapless::String::new();
if let Some(name) = persisted_name.as_deref() {
// Cap at heapless::String<32>'s capacity. Operator
// names longer than the BLE GAP 22-byte limit will be
// rejected by the advertise layer; here we just keep
// the buffer write infallible.
let n = name.len().min(32);
if let Err(e) = s.push_str(&name[..n]) {
defmt::panic!("ble: name copy failed: {}", defmt::Debug2Format(&e));
// BLE peripheral — brought up ONLY when no Wi-Fi SSID is configured.
// BLE (Hardware Buddy bridge) and Wi-Fi (HTTP/MCP plane) are mutually
// exclusive: running both needs radio coexistence, whose buffers plus
// BLE's own exhaust the scarce internal-SRAM heap and make the Wi-Fi MAC
// fail to start with `NoMem`. With an SSID present we skip BLE so the
// Wi-Fi stack gets that internal RAM. Address + name derive from the
// eFuse MAC for a stable cross-reboot identity.
if net_config.wifi.ssid.trim().is_empty() {
#[allow(clippy::items_after_statements)]
static BLE_NAME: StaticCell<heapless::String<32>> = StaticCell::new();
let ble_mac = esp_hal::efuse::Efuse::mac_address();
// First, try the operator-supplied name from
// `/sd/DEVICE.NAM` (set via the Hardware Buddy `cmd:name`
// message). Fall back to the MAC-suffixed default. The name
// must start with `Claude` so the Hardware Buddy picker filters
// us in; the default does, and a desktop-set override is
// assumed to as well — we don't enforce the prefix.
let persisted_name = stackchan_firmware::storage::with_storage(
stackchan_firmware::storage::FirmwareStorage::read_device_name,
)
.await
.and_then(|r| match r {
Ok(opt) => opt,
Err(e) => {
defmt::warn!(
"ble: device name read failed ({}); using MAC-suffixed default",
defmt::Debug2Format(&e)
);
None
}
});
let ble_name: &'static str = {
use core::fmt::Write as _;
let mut s: heapless::String<32> = heapless::String::new();
if let Some(name) = persisted_name.as_deref() {
// Cap at heapless::String<32>'s capacity. Operator
// names longer than the BLE GAP 22-byte limit will be
// rejected by the advertise layer; here we just keep
// the buffer write infallible.
let n = name.len().min(32);
if let Err(e) = s.push_str(&name[..n]) {
defmt::panic!("ble: name copy failed: {}", defmt::Debug2Format(&e));
}
} else if let Err(e) = write!(
&mut s,
"Claude stk-{:02x}{:02x}{:02x}",
ble_mac[3], ble_mac[4], ble_mac[5]
) {
defmt::panic!("ble: name format failed: {}", defmt::Debug2Format(&e));
}
} else if let Err(e) = write!(
&mut s,
"Claude stk-{:02x}{:02x}{:02x}",
ble_mac[3], ble_mac[4], ble_mac[5]
BLE_NAME.init(s).as_str()
};
defmt::info!("ble: advertising as {=str}", ble_name);
let ble_connector = match esp_radio::ble::controller::BleConnector::new(
radio,
peripherals.BT,
esp_radio::ble::Config::default(),
) {
defmt::panic!("ble: name format failed: {}", defmt::Debug2Format(&e));
Ok(c) => c,
Err(e) => defmt::panic!(
"ble: BleConnector::new failed ({})",
defmt::Debug2Format(&e)
),
};
let ble_controller: trouble_host::prelude::ExternalController<_, 20> =
trouble_host::prelude::ExternalController::new(ble_connector);
if let Err(e) = spawner.spawn(ble::ble_task(ble::BleTaskConfig {
controller: ble_controller,
address_bytes: ble_mac,
local_name: ble_name,
})) {
defmt::panic!("spawn(ble_task) failed: {}", defmt::Debug2Format(&e));
}
BLE_NAME.init(s).as_str()
};
defmt::info!("ble: advertising as {=str}", ble_name);
let ble_connector = match esp_radio::ble::controller::BleConnector::new(
radio,
peripherals.BT,
esp_radio::ble::Config::default(),
) {
Ok(c) => c,
Err(e) => defmt::panic!(
"ble: BleConnector::new failed ({})",
defmt::Debug2Format(&e)
),
};
let ble_controller: trouble_host::prelude::ExternalController<_, 20> =
trouble_host::prelude::ExternalController::new(ble_connector);
if let Err(e) = spawner.spawn(ble::ble_task(ble::BleTaskConfig {
controller: ble_controller,
address_bytes: ble_mac,
local_name: ble_name,
})) {
defmt::panic!("spawn(ble_task) failed: {}", defmt::Debug2Format(&e));
} else {
defmt::info!(
"ble: skipped — Wi-Fi SSID configured; BLE and Wi-Fi are mutually exclusive to fit internal SRAM"
);
}
Comment on lines +1581 to 1651
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 BLE provisioning → Wi-Fi soft-reconfig still coexists at runtime

The boot-time exclusion correctly prevents the NoMem scenario when an SSID is already in STACKCHAN.RON. However, when a device first boots with no SSID (BLE starts), a Hardware Buddy provisioning write fires WIFI_RECONFIG, and wifi_task calls controller.start_async() while BLE is still running — the same memory-pressure path that caused the original failure. This was broken before this PR too, so no regression is introduced here, but the comment "provisioning over BLE is unaffected" may set a false expectation: BLE advertising/GATT work fine, but the subsequent Wi-Fi bring-up in that flow is still subject to the NoMem constraint until a reboot (or the coex feature is dropped as noted in the follow-ups).

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/stackchan-firmware/src/main.rs
Line: 1581-1651

Comment:
**BLE provisioning → Wi-Fi soft-reconfig still coexists at runtime**

The boot-time exclusion correctly prevents the NoMem scenario when an SSID is already in `STACKCHAN.RON`. However, when a device first boots with no SSID (BLE starts), a Hardware Buddy provisioning write fires `WIFI_RECONFIG`, and `wifi_task` calls `controller.start_async()` while BLE is still running — the same memory-pressure path that caused the original failure. This was broken before this PR too, so no regression is introduced here, but the comment "provisioning over BLE is unaffected" may set a false expectation: BLE advertising/GATT work fine, but the subsequent Wi-Fi bring-up in that flow is still subject to the NoMem constraint until a reboot (or the `coex` feature is dropped as noted in the follow-ups).

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code


if let Err(e) = spawner.spawn(stackchan_firmware::desktop_permission::desktop_permission_task())
Expand Down
14 changes: 10 additions & 4 deletions crates/stackchan-firmware/src/net/stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,16 @@ use embassy_net::{Runner, StackResources};
use esp_radio::wifi::WifiDevice;
use static_cell::StaticCell;

/// Maximum number of concurrent sockets the firmware ever needs:
/// one for SNTP (UDP), one or two for the HTTP server (listen +
/// accepted), one for mDNS responder, plus a couple of slack.
pub const STACK_SOCKETS: usize = 6;
/// Maximum number of concurrent sockets the firmware ever needs.
///
/// `smoltcp` panics ("adding a socket to a full `SocketSet`") the moment
/// demand exceeds this, so it must cover every consumer that can be live at
/// once: `HTTP_WORKER_COUNT` HTTP listeners (each owns its own socket), plus
/// mDNS, SNTP, the DHCP + DNS clients, and the optional outbound clients
/// (agent sidecar, `VoiceVox`, audio debug). The old value of 6 pre-dated the
/// 4-worker HTTP pool and overflowed as soon as Wi-Fi linked. Slots are cheap
/// static metadata, so over-provisioning costs only a few `.bss` bytes.
pub const STACK_SOCKETS: usize = crate::net::http::HTTP_WORKER_COUNT + 8;

/// Static cell for the embassy-net resource pool. Sized at compile
/// time to avoid heap fragmentation under SNTP/HTTP/mDNS churn.
Expand Down
3 changes: 2 additions & 1 deletion crates/stackchan-firmware/src/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,8 @@ where
let dst = root
.open_file_in_dir(&short, Mode::ReadWriteCreateOrTruncate)
.map_err(|_| StorageError::Write)?;
dst.write(rendered.as_bytes()).map_err(|_| StorageError::Write)?;
dst.write(rendered.as_bytes())
.map_err(|_| StorageError::Write)?;
dst.flush().map_err(|_| StorageError::Write)?;
}
// Staging served its purpose (a crash-safe copy existed before we
Expand Down
Loading