diff --git a/CMakeLists.txt b/CMakeLists.txt index 07fd427..f43abc4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -61,6 +61,11 @@ option(SWITCH_PICO_CYW43_PACKET_READ "Use packet-level CYW43 receive transactions" ${SWITCH_PICO_NATIVE_DEFAULT}) option(SWITCH_PICO_HCI_CREDIT_BATCH "Batch incoming HCI credit returns" ${SWITCH_PICO_NATIVE_DEFAULT}) +option(SWITCH_PICO_HCI_CREDIT_BUFFER + "Return receive credits independently of the ACL packet buffer" ${SWITCH_PICO_HCI_CREDIT_BATCH}) +if(SWITCH_PICO_HCI_CREDIT_BUFFER AND NOT SWITCH_PICO_HCI_CREDIT_BATCH) + message(FATAL_ERROR "The dedicated HCI credit buffer requires credit batching") +endif() if(SWITCH_PICO_HD_RUMBLE) set(SWITCH_PICO_HAPTICS_EXPERIMENT ON) endif() @@ -309,6 +314,9 @@ if(SWITCH_PICO_INPUT_BACKEND STREQUAL "BLUEPAD32") if(SWITCH_PICO_HCI_CREDIT_BATCH) target_compile_definitions(switch-pico PRIVATE SWITCH_PICO_HCI_CREDIT_BATCH=1) endif() + if(SWITCH_PICO_HCI_CREDIT_BUFFER) + target_compile_definitions(switch-pico PRIVATE SWITCH_PICO_HCI_CREDIT_BUFFER=1) + endif() if(SWITCH_PICO_HAPTICS_EXPERIMENT) target_sources(switch-pico PRIVATE ${SWITCH_PICO_SOURCE_DIR}/input/haptics_experiment.cpp diff --git a/HAPTICS_EXPERIMENT.md b/HAPTICS_EXPERIMENT.md index ed8ac3e..ef6ebb4 100644 --- a/HAPTICS_EXPERIMENT.md +++ b/HAPTICS_EXPERIMENT.md @@ -307,6 +307,37 @@ profiles were compared with the pre-migration backup; the temporary editor profile and name were restored. No configuration, bond, or wake-identity reset was part of the transport work. +### Dedicated receive-credit buffer + +`SWITCH_PICO_HCI_CREDIT_BUFFER` defaults to the credit-batching setting, so normal +AIO and automatic Switch/XInput builds enable it. Setting it to `OFF` retains +the original batched/shared-buffer path for A/B comparisons. Enabling it requires +credit batching; UART defaults remain unchanged. + +The HCI patch uses a separate 24-byte, word-aligned buffer with the current +four-connection configuration, including CYW43's four-byte transport header. +Ready receive-credit commands precede pending ACL continuations without bypassing +transport readiness or an in-flight fragment. Their completion releases only +the credit buffer. Transfer ownership survives batching cancellation until +completion or transport close. A send returning after stack reinitialization +cannot release the new stack's buffer. + +A native probe delivered three incoming packets while the outgoing buffer +remained reserved. At 20 ms, the shared path retained all three receive credits; +the dedicated path had returned all three in two commands without releasing +the outgoing buffer. Regressions cover mixed BLE input/Classic fragmentation, +synchronous, delayed and CYW43-style inline completion, buffer lifetime, sleep, +power-cycle recovery and the unchanged default/shared paths. AddressSanitizer +and UndefinedBehaviorSanitizer also passed. + +The automatic-mode image was flashed and verified on hardware. Configuration +generation 21 / CRC `b58672ac`, all nine pairings, the profile catalog and active +profile selections were unchanged. Two controllers supplied input, but the +DualSense native stream hit its can-send timeout both with this change and with +the previous packaged firmware. The host-side blockage is fixed; no radio, +native-stream reliability or physical latency improvement is established by +that comparison. + ### Historical mixed-controller cadence limit Final testing with a Switch Pro plus a DualSense and continuous USB motion diff --git a/README.md b/README.md index 7273fd8..cb19431 100644 --- a/README.md +++ b/README.md @@ -336,6 +336,14 @@ still reads older 32-byte responses and treats their missing counters as unreported, not zero. These count firmware queue discards/rejections, not physical actuator-delivery receipts. +Receive-credit commands use a separate, word-aligned buffer so pending outgoing +ACL fragments cannot block receive-credit returns. This is enabled by default +with credit batching; `-DSWITCH_PICO_HCI_CREDIT_BUFFER=OFF` restores the shared +buffer for comparison. It does not change radio scheduling, pairing policy, or +the controller's credit limits. See the +[credit-buffer results](HAPTICS_EXPERIMENT.md#dedicated-receive-credit-buffer) +for the distinction between host-side progress and measured gameplay latency. + ### Per-controller profiles @@ -496,21 +504,23 @@ results do **not** establish lossless arbitrary workloads or perceptual equivalence. All 40 profiles, names, active selections and adapter configuration were preserved during this upgrade. -**Joy-Con 2 connection timing:** both halves now request **7.5 ms BLE connection -intervals**, in Paired and Individual modes, including while Classic controllers -are connected. This deliberately replaces their earlier automatic 30 ms slowdown -to favor Joy-Con responsiveness and rumble quality. Other Switch 2 models retain -the existing coexistence policy: 30 ms when at least two physical Switch 2 links -are present, otherwise 7.5 ms. Unrelated BLE controllers are not retimed. Bonds, -profiles, HD encoding and the DualSense timeout are unchanged. Negotiated -intervals are reconciled after asynchronous updates and topology changes. +**Switch 2 connection timing:** all supported Switch 2 BLE controllers now request +**7.5 ms connection intervals**, including Switch 2 Pro and both Joy-Con 2 halves +in Paired or Individual mode. The earlier 30 ms policy for multiple Switch 2 links +is removed; additional BLE or Classic controllers do not slow these requests. +Unrelated BLE controllers are not retimed. Bonds, profiles, HD encoding and the +DualSense timeout are unchanged. Initial setup retains ownership of its interval +request; ready links reconcile negotiated intervals with at most one retry per +second. More frequent connection events favor responsiveness but can increase +shared-radio contention; this does not establish lower measured gameplay latency. The BLE connection interval is not the rumble packet cadence: multiple packets can travel per connection event. The normal active-output algorithm is unchanged by this checkpoint; the direct-packet lab fixtures are not release features. Lifecycle coverage includes Paired/Individual mode changes, Classic arrival and -departure, handle reuse, correcting a slow negotiated Joy-Con interval, unrelated -BLE isolation, and the retained Switch 2 Pro interval/retry policy. +departure, handle reuse, multiple Switch 2 Pro links, correcting slow negotiated +intervals, unrelated BLE isolation, asynchronous settlement across clock wrap +and bounded retries after rejected requests. **Idle Switch 2 rumble traffic:** the parser sends three successful neutral writes, then suppresses further idle output. Any successful non-neutral packet diff --git a/firmware/switch-pico-adapter-feasibility.elf b/firmware/switch-pico-adapter-feasibility.elf index 65b0fdf..031d9ff 100755 Binary files a/firmware/switch-pico-adapter-feasibility.elf and b/firmware/switch-pico-adapter-feasibility.elf differ diff --git a/firmware/switch-pico-adapter-feasibility.uf2 b/firmware/switch-pico-adapter-feasibility.uf2 index 7ab9b75..e3b2671 100644 Binary files a/firmware/switch-pico-adapter-feasibility.uf2 and b/firmware/switch-pico-adapter-feasibility.uf2 differ diff --git a/firmware/switch-pico-aio.elf b/firmware/switch-pico-aio.elf index 639b4e9..214746b 100755 Binary files a/firmware/switch-pico-aio.elf and b/firmware/switch-pico-aio.elf differ diff --git a/firmware/switch-pico-aio.uf2 b/firmware/switch-pico-aio.uf2 index 7ab9b75..e3b2671 100644 Binary files a/firmware/switch-pico-aio.uf2 and b/firmware/switch-pico-aio.uf2 differ diff --git a/patches/btstack-credit-batch.patch b/patches/btstack-credit-batch.patch index 8a53b9f..d3b4f52 100644 --- a/patches/btstack-credit-batch.patch +++ b/patches/btstack-credit-batch.patch @@ -1,6 +1,6 @@ --- a/lib/btstack/src/hci.c +++ b/lib/btstack/src/hci.c -@@ -93,6 +93,12 @@ +@@ -93,6 +93,20 @@ #endif #endif @@ -9,11 +9,19 @@ +#error "SWITCH_PICO_HCI_CREDIT_BATCH requires controller-to-host flow control with three ACL credits" +#endif +#endif ++#ifdef SWITCH_PICO_HCI_CREDIT_BUFFER ++#ifndef SWITCH_PICO_HCI_CREDIT_BATCH ++#error "SWITCH_PICO_HCI_CREDIT_BUFFER requires SWITCH_PICO_HCI_CREDIT_BATCH" ++#endif ++#if !defined(MAX_NR_HCI_CONNECTIONS) || (MAX_NR_HCI_CONNECTIONS < 1) || (MAX_NR_HCI_CONNECTIONS > 63) ++#error "SWITCH_PICO_HCI_CREDIT_BUFFER requires 1..63 configured HCI connections" ++#endif ++#endif + #ifndef MAX_NR_CONTROLLER_ACL_BUFFERS #define MAX_NR_CONTROLLER_ACL_BUFFERS 255 #endif -@@ -273,6 +279,74 @@ +@@ -273,6 +287,84 @@ #endif static hci_stack_t * hci_stack = NULL; @@ -23,6 +31,13 @@ +static bool hci_credit_batch_due; +static bool hci_credit_batch_enabled; +static uint32_t hci_credit_batch_generation; ++#ifdef SWITCH_PICO_HCI_CREDIT_BUFFER ++static bool hci_credit_batch_in_flight; ++// Independent of the prepared ACL buffer, including transport header and word padding. ++#define HCI_CREDIT_BATCH_PRE_BUFFER_SIZE ((HCI_OUTGOING_PRE_BUFFER_SIZE + 3u) & ~3u) ++static uint32_t hci_credit_batch_buffer[ ++ (HCI_CREDIT_BATCH_PRE_BUFFER_SIZE + 4u + 4u * MAX_NR_HCI_CONNECTIONS) / sizeof(uint32_t)]; ++#endif + +static void hci_credit_batch_reset(void){ + if (hci_credit_batch_armed){ @@ -34,7 +49,10 @@ + +static void hci_credit_batch_stop(void){ + hci_credit_batch_enabled = false; ++#ifndef SWITCH_PICO_HCI_CREDIT_BUFFER + hci_credit_batch_generation++; ++#endif ++ // Cancellation does not end an asynchronous transfer; completion or close does. + hci_credit_batch_reset(); +} + @@ -88,7 +106,27 @@ #ifdef ENABLE_CLASSIC // default name static const char * default_classic_name = "BTstack 00:00:00:00:00:00"; -@@ -1195,6 +1269,9 @@ +@@ -738,6 +830,9 @@ + + // only used to send HCI Host Number Completed Packets + static int hci_can_send_command_packet_transport(void){ ++#ifdef SWITCH_PICO_HCI_CREDIT_BUFFER ++ if (hci_credit_batch_in_flight) return 0; ++#endif + if (hci_stack->hci_packet_buffer_reserved) return 0; + + // check for async hci transport implementations +@@ -756,6 +851,9 @@ + } + + static int hci_transport_can_send_prepared_packet_now(uint8_t packet_type){ ++#ifdef SWITCH_PICO_HCI_CREDIT_BUFFER ++ if (hci_credit_batch_in_flight) return false; ++#endif + // check for async hci transport implementations + if (!hci_stack->hci_transport->can_send_packet_now) return true; + return hci_stack->hci_transport->can_send_packet_now(packet_type); +@@ -1195,6 +1293,9 @@ #ifdef ENABLE_HCI_CONTROLLER_TO_HOST_FLOW_CONTROL hci_stack->host_completed_packets = 1; conn->num_packets_completed++; @@ -98,7 +136,7 @@ #endif // handle different packet types -@@ -1269,8 +1346,10 @@ +@@ -1269,8 +1370,10 @@ return; } @@ -109,7 +147,7 @@ } static void hci_connection_stop_timer(hci_connection_t * conn){ -@@ -1296,6 +1375,9 @@ +@@ -1296,6 +1399,9 @@ btstack_linked_list_remove(&hci_stack->connections, (btstack_linked_item_t *) conn); btstack_memory_hci_connection_free( conn ); @@ -119,7 +157,7 @@ // now it's gone hci_emit_nr_connections_changed(); -@@ -4314,6 +4396,12 @@ +@@ -4314,6 +4420,12 @@ conn = hci_connection_for_handle(handle); if (!conn) break; @@ -132,7 +170,21 @@ #ifdef ENABLE_CLASSIC // pairing failed if it was ongoing hci_pairing_complete(conn, ERROR_CODE_REMOTE_USER_TERMINATED_CONNECTION); -@@ -4780,6 +4868,10 @@ +@@ -4367,6 +4479,13 @@ + log_error("Synchronous HCI Transport shouldn't send HCI_EVENT_TRANSPORT_PACKET_SENT"); + return; // instead of break: to avoid re-entering hci_run() + } ++#ifdef SWITCH_PICO_HCI_CREDIT_BUFFER ++ if (hci_credit_batch_in_flight){ ++ hci_credit_batch_in_flight = false; ++ // This completion owns only the credit buffer, not a prepared ACL fragment. ++ break; ++ } ++#endif + hci_stack->acl_fragmentation_tx_active = 0; + #ifdef ENABLE_LE_ISOCHRONOUS_STREAMS + hci_stack->iso_fragmentation_tx_active = 0; +@@ -4780,6 +4899,10 @@ #ifdef ENABLE_HCI_CONTROLLER_TO_HOST_FLOW_CONTROL conn->num_packets_completed++; hci_stack->host_completed_packets = 1; @@ -143,7 +195,7 @@ hci_run(); #endif } -@@ -4814,6 +4906,10 @@ +@@ -4814,6 +4937,10 @@ break; case HCI_ACL_DATA_PACKET: acl_handler(packet, size); @@ -154,19 +206,22 @@ break; #ifdef ENABLE_CLASSIC case HCI_SCO_DATA_PACKET: -@@ -4869,6 +4965,11 @@ +@@ -4869,6 +4996,14 @@ #endif static void hci_state_reset(void){ +#ifdef SWITCH_PICO_HCI_CREDIT_BATCH + hci_credit_batch_reset(); + hci_credit_batch_generation++; ++#ifdef SWITCH_PICO_HCI_CREDIT_BUFFER ++ hci_credit_batch_in_flight = false; ++#endif + hci_stack->host_completed_packets = 0; +#endif // no connections yet hci_stack->connections = NULL; -@@ -4953,6 +5054,10 @@ +@@ -4953,6 +5088,10 @@ #endif void hci_init(const hci_transport_t *transport, const void *config){ @@ -177,7 +232,7 @@ #ifdef HAVE_MALLOC if (!hci_stack) { -@@ -5082,6 +5187,9 @@ +@@ -5082,6 +5221,9 @@ } void hci_deinit(void){ @@ -187,7 +242,7 @@ btstack_run_loop_remove_timer(&hci_stack->timeout); #ifdef HAVE_MALLOC if (hci_stack) { -@@ -5138,6 +5246,9 @@ +@@ -5138,6 +5280,9 @@ } void hci_close(void){ @@ -197,7 +252,7 @@ #ifdef ENABLE_CLASSIC // close remote device db -@@ -5264,6 +5375,10 @@ +@@ -5264,6 +5409,10 @@ // HCI_STATE_FALLING_ASLEEP on open static int hci_power_control_on(void){ @@ -208,7 +263,7 @@ // power on int err = 0; -@@ -5300,6 +5415,9 @@ +@@ -5300,11 +5449,18 @@ } static void hci_power_control_off(void){ @@ -218,7 +273,16 @@ log_info("hci_power_control_off"); -@@ -5319,6 +5437,9 @@ + // close low-level device + hci_stack->hci_transport->close(); ++#ifdef SWITCH_PICO_HCI_CREDIT_BUFFER ++ hci_credit_batch_in_flight = false; ++ hci_credit_batch_generation++; ++#endif + + log_info("hci_power_control_off - hci_transport closed"); + +@@ -5319,6 +5475,9 @@ } static void hci_power_control_sleep(void){ @@ -228,7 +292,7 @@ log_info("hci_power_control_sleep"); -@@ -5363,6 +5484,10 @@ +@@ -5363,6 +5522,10 @@ } static void hci_power_enter_initializing_state(void){ @@ -239,7 +303,7 @@ // set up state machine hci_stack->num_cmd_packets = 1; // assume that one cmd can be sent hci_stack->hci_packet_buffer_reserved = false; -@@ -5543,6 +5668,10 @@ +@@ -5543,6 +5706,10 @@ } int hci_power_control(HCI_POWER_MODE power_mode){ @@ -250,7 +314,7 @@ log_info("hci_power_control: %d, current mode %u", power_mode, hci_stack->state); btstack_run_loop_remove_timer(&hci_stack->timeout); int err = 0; -@@ -5834,6 +5963,11 @@ +@@ -5834,10 +6001,19 @@ #ifdef ENABLE_HCI_CONTROLLER_TO_HOST_FLOW_CONTROL static void hci_host_num_completed_packets(void){ @@ -261,9 +325,32 @@ +#endif // create packet manually as arrays are not supported and num_commands should not get reduced ++#ifdef SWITCH_PICO_HCI_CREDIT_BUFFER ++ uint8_t * packet = (uint8_t *) hci_credit_batch_buffer + HCI_CREDIT_BATCH_PRE_BUFFER_SIZE; ++#else hci_reserve_packet_buffer(); -@@ -5868,6 +6002,9 @@ + uint8_t * packet = hci_get_outgoing_packet_buffer(); ++#endif + uint16_t size = 0; + uint16_t num_handles = 0; +@@ -5851,6 +6027,9 @@ + for (it = (btstack_linked_item_t *) hci_stack->connections; it ; it = it->next){ + hci_connection_t * connection = (hci_connection_t *) it; + if (connection->num_packets_completed){ ++#ifdef SWITCH_PICO_HCI_CREDIT_BUFFER ++ btstack_assert(num_handles < MAX_NR_HCI_CONNECTIONS); ++#endif + little_endian_store_16(packet, size, connection->con_handle); + size += 2; + little_endian_store_16(packet, size, connection->num_packets_completed); +@@ -5866,12 +6045,22 @@ + + hci_stack->host_completed_packets = 0; + ++#ifdef SWITCH_PICO_HCI_CREDIT_BUFFER ++ hci_credit_batch_in_flight = true; ++#endif hci_dump_packet(HCI_COMMAND_DATA_PACKET, 0, packet, size); hci_stack->hci_transport->send_packet(HCI_COMMAND_DATA_PACKET, packet, size); +#ifdef SWITCH_PICO_HCI_CREDIT_BATCH @@ -272,10 +359,41 @@ // release packet buffer for synchronous transport implementations if (hci_transport_synchronous()){ -@@ -7664,12 +7801,21 @@ ++#ifdef SWITCH_PICO_HCI_CREDIT_BUFFER ++ hci_credit_batch_in_flight = false; ++#else + hci_release_packet_buffer(); ++#endif + hci_emit_transport_packet_sent(); + } + } +@@ -7652,6 +7841,21 @@ + return; + } + ++#ifdef SWITCH_PICO_HCI_CREDIT_BUFFER ++ if (hci_credit_batch_in_flight) return; ++ // Return receive credits before continuing an ACL packet that may exhaust ++ // transmit credits. A prepared packet is not an occupied transport. ++ if (hci_stack->host_completed_packets && hci_credit_batch_ready()){ ++ if (!hci_transport_synchronous() && hci_stack->acl_fragmentation_tx_active) return; ++#ifdef ENABLE_LE_ISOCHRONOUS_STREAMS ++ if (hci_stack->iso_fragmentation_tx_active) return; ++#endif ++ if (!hci_transport_can_send_prepared_packet_now(HCI_COMMAND_DATA_PACKET)) return; ++ hci_host_num_completed_packets(); ++ return; ++ } ++#endif ++ + bool done; + + // send continuation fragments first, as they block the prepared packet buffer +@@ -7664,12 +7868,23 @@ #endif #ifdef ENABLE_HCI_CONTROLLER_TO_HOST_FLOW_CONTROL ++#ifndef SWITCH_PICO_HCI_CREDIT_BUFFER +#ifdef SWITCH_PICO_HCI_CREDIT_BATCH + // A single deferred ACL credit must not block ordinary HCI commands. + if (hci_stack->host_completed_packets && hci_credit_batch_ready()){ @@ -290,6 +408,7 @@ hci_host_num_completed_packets(); return; } ++#endif +#endif #endif diff --git a/src/firmware/input/bluepad32_input_backend.cpp b/src/firmware/input/bluepad32_input_backend.cpp index 6f58c07..41d0a61 100644 --- a/src/firmware/input/bluepad32_input_backend.cpp +++ b/src/firmware/input/bluepad32_input_backend.cpp @@ -51,9 +51,8 @@ constexpr uint32_t kDefaultPairingWindowDurationMs = constexpr uint32_t kPairingResetFeedbackDurationMs = 2000; // Bluetooth Classic units are 0.625 ms: 0x1900 = 4 seconds. constexpr uint16_t kClassicLinkSupervisionTimeout = 0x1900; -// LE units are 1.25 ms. Joy-Cons retain the fast interval even with Classic. +// LE units are 1.25 ms. All Switch 2 links request the 7.5 ms minimum. constexpr uint16_t kSwitch2FastInterval = 6; -constexpr uint16_t kSwitch2MixedInterval = 24; constexpr uint32_t kSwitch2IntervalSettleMs = 1000; constexpr uint8_t kAllBlePairingMethods = SM_STK_GENERATION_METHOD_JUST_WORKS | @@ -602,11 +601,9 @@ void stop_background_scan() { g_background_scan_active = false; } } -// Core 1 only. Count physical links, not logical players. Joy-Cons always keep -// their fast default; retain preconnection coexistence timing for other Switch 2 -// models when multiple physical Switch 2 links share the radio. +// Core 1 only. Reconcile every ready physical Switch 2 link to the fast interval, +// independently of player grouping, controller count, or Classic connections. void apply_radio_connection_policy() { - unsigned switch2_links = 0; uni_hid_device_t* ready[kSlotCount]{}; for (const BackendSlot& slot : g_slots) { uni_hid_device_t* targets[] = {slot.device, slot.companion}; @@ -616,7 +613,6 @@ void apply_radio_connection_policy() { const auto type = gap_get_connection_type(target->conn.handle); if (type == GAP_CONNECTION_LE && uni_hid_parser_switch2_is_ble_device(target)) { - ++switch2_links; // The parser requests its initial interval during setup. // Do not race that request by changing a pending device here. if (slot.active) ready[index] = target; @@ -630,24 +626,22 @@ void apply_radio_connection_policy() { request = {}; continue; } - const uint16_t desired = switch2_links >= 2 && joycon_side(ready[index]) == 0 - ? kSwitch2MixedInterval - : kSwitch2FastInterval; const auto handle = ready[index]->conn.handle; if (request.handle != handle) request = {}; const uint16_t actual = gap_le_connection_interval(handle); if (request.interval != 0 && actual != request.interval && now_ms - request.requested_ms < kSwitch2IntervalSettleMs) { - // Let an accepted asynchronous update settle before reversing it. + // Let an accepted asynchronous update settle before retrying it. // API success alone does not prove that negotiation completed. continue; } request.interval = 0; - if (actual == desired) continue; - gap_update_connection_parameters(handle, desired, desired, 0, 600); + if (actual == kSwitch2FastInterval) continue; + gap_update_connection_parameters( + handle, kSwitch2FastInterval, kSwitch2FastInterval, 0, 600); // Reconcile negotiated state on the configuration timer. Rejected or // incomplete requests are retried at most once per second per link. - request = {handle, desired, now_ms}; + request = {handle, kSwitch2FastInterval, now_ms}; } } diff --git a/tests/bluepad32_backend_lifecycle_test.cpp b/tests/bluepad32_backend_lifecycle_test.cpp index e70a702..b7f709b 100644 --- a/tests/bluepad32_backend_lifecycle_test.cpp +++ b/tests/bluepad32_backend_lifecycle_test.cpp @@ -3061,6 +3061,7 @@ void test_switch2_radio_policy(bool individual) { auto right = switch2_device(1, UNI_SW2_JOYCON_R_PID); auto classic = device(2, true, UNI_BT_CONN_PROTOCOL_BR_EDR); auto other_ble = device(3, true, UNI_BT_CONN_PROTOCOL_BLE); + negotiated_intervals[other_ble.conn.handle] = 24; ready_switch2(left); require(negotiated_intervals[left.conn.handle] == 6, "a single Switch2 link must retain fast scheduling"); @@ -3075,7 +3076,7 @@ void test_switch2_radio_policy(bool individual) { platform_on_device_connected(&classic); require(negotiated_intervals[left.conn.handle] == 6 && negotiated_intervals[right.conn.handle] == 6 && - negotiated_intervals[other_ble.conn.handle] == 6, + negotiated_intervals[other_ble.conn.handle] == 24, "Classic setup must not slow Joy-Cons or retime unrelated BLE"); require(platform_on_device_ready(&classic) == UNI_ERROR_SUCCESS, "Classic controller must complete setup alongside the pair"); @@ -3105,10 +3106,19 @@ void test_switch2_radio_policy(bool individual) { "peer-negotiated slow Joy-Con links must reconcile to the fast default"); platform_on_device_disconnected(&right); right = switch2_device(1, UNI_SW2_PRO_PID); + negotiated_intervals[right.conn.handle] = 24; ready_switch2(right); require(negotiated_intervals[left.conn.handle] == 6 && - negotiated_intervals[right.conn.handle] == 24, - "Joy-Con preference must not remove the existing Switch2 Pro coexistence policy"); + negotiated_intervals[right.conn.handle] == 6 && + negotiated_intervals[other_ble.conn.handle] == 24, + "Switch2 Pro must join at the fast interval without retiming unrelated BLE"); + platform_on_device_disconnected(&left); + left = switch2_device(0, UNI_SW2_PRO_PID); + negotiated_intervals[left.conn.handle] = 24; + ready_switch2(left); + require(negotiated_intervals[left.conn.handle] == 6 && + negotiated_intervals[right.conn.handle] == 6, + "multiple Switch2 Pro links must remain fast alongside Classic"); } void test_switch2_radio_settling() { @@ -3119,39 +3129,50 @@ void test_switch2_radio_settling() { platform_on_device_connected(&classic); require(platform_on_device_ready(&classic) == UNI_ERROR_SUCCESS, "Classic-first connection must become ready"); - ready_switch2(left); + negotiated_intervals[left.conn.handle] = 24; + negotiated_intervals[right.conn.handle] = 24; defer_interval_updates = true; now_ms = UINT32_MAX - 10; + ready_switch2(left); platform_on_device_connected(&right); require(interval_requests[right.conn.handle] == 0, "pending Switch2 setup must retain ownership of its initial interval"); require(platform_on_device_ready(&right) == UNI_ERROR_SUCCESS, "second Switch2 connection must become ready"); - platform_on_device_disconnected(&right); - // The surviving controller completes the old update after losing its mate. - for (auto* half : {&left, &right}) { - negotiated_intervals[half->conn.handle] = pending_intervals[half->conn.handle]; - } - now_ms += 50; - process_configuration_timer(&g_configuration_timer); - negotiated_intervals[left.conn.handle] = pending_intervals[left.conn.handle]; - require(negotiated_intervals[left.conn.handle] == 6, - "late coexistence update must not strand a solo at slow intervals across clock wrap"); - process_configuration_timer(&g_configuration_timer); - defer_interval_updates = false; - reject_interval_updates = true; - ready_switch2(right); - const unsigned attempts = interval_requests[left.conn.handle]; + require(pending_intervals[left.conn.handle] == 6 && + pending_intervals[right.conn.handle] == 6, + "both slow Switch2 Pro links must request the fast interval"); + const unsigned deferred_attempts = interval_requests[left.conn.handle]; now_ms += 999; process_configuration_timer(&g_configuration_timer); - require(interval_requests[left.conn.handle] == attempts, - "rejected negotiation must not flood the HCI command queue"); + require(interval_requests[left.conn.handle] == deferred_attempts && + negotiated_intervals[left.conn.handle] == 24, + "an accepted asynchronous request must be allowed to settle across clock wrap"); + platform_on_device_disconnected(&right); + negotiated_intervals[left.conn.handle] = pending_intervals[left.conn.handle]; + process_configuration_timer(&g_configuration_timer); + require(negotiated_intervals[left.conn.handle] == 6 && + interval_requests[left.conn.handle] == deferred_attempts, + "a late fast-interval acknowledgement must survive another controller leaving"); + defer_interval_updates = false; + reject_interval_updates = true; + negotiated_intervals[left.conn.handle] = 24; + right = switch2_device(1, UNI_SW2_PRO_PID); + negotiated_intervals[right.conn.handle] = 24; + ready_switch2(right); + const unsigned left_attempts = interval_requests[left.conn.handle]; + const unsigned right_attempts = interval_requests[right.conn.handle]; + now_ms += 999; + process_configuration_timer(&g_configuration_timer); + require(interval_requests[left.conn.handle] == left_attempts && + interval_requests[right.conn.handle] == right_attempts, + "rejected negotiations must not flood the HCI command queue"); reject_interval_updates = false; ++now_ms; process_configuration_timer(&g_configuration_timer); - require(negotiated_intervals[left.conn.handle] == 24 && - negotiated_intervals[right.conn.handle] == 24, - "transiently rejected negotiation must recover without reconnect or pairing"); + require(negotiated_intervals[left.conn.handle] == 6 && + negotiated_intervals[right.conn.handle] == 6, + "transiently rejected fast intervals must recover without reconnect or pairing"); } void test_switch2_mate_reconnect() { diff --git a/tests/btstack_credit_batch_native_stubs/btstack_config.h b/tests/btstack_credit_batch_native_stubs/btstack_config.h index 8b04cd4..21e61f7 100644 --- a/tests/btstack_credit_batch_native_stubs/btstack_config.h +++ b/tests/btstack_credit_batch_native_stubs/btstack_config.h @@ -2,9 +2,14 @@ #define SWITCH_PICO_CREDIT_BATCH_TEST_CONFIG_H #define ENABLE_CLASSIC +#define ENABLE_BLE +#define ENABLE_LE_CENTRAL +#define ENABLE_LE_PERIPHERAL #define ENABLE_HCI_CONTROLLER_TO_HOST_FLOW_CONTROL #define ENABLE_BTSTACK_ASSERT #define HAVE_MALLOC +#define HCI_OUTGOING_PRE_BUFFER_SIZE 4 +#define MAX_NR_HCI_CONNECTIONS 4 #define HCI_ACL_PAYLOAD_SIZE 1021 #define HCI_HOST_ACL_PACKET_LEN 1021 #define HCI_HOST_ACL_PACKET_NUM 3 diff --git a/tests/btstack_credit_batch_test.c b/tests/btstack_credit_batch_test.c index 7a8eb2b..a8486ca 100644 --- a/tests/btstack_credit_batch_test.c +++ b/tests/btstack_credit_batch_test.c @@ -14,6 +14,14 @@ static unsigned wire_count; static uint8_t wire[32][64]; static int wire_size[32]; static void (*during_send)(void); +static uint8_t wire_type[32]; +static bool complete_inline; +static uint8_t *last_transport_packet; + +static void transport_sent(void){ + uint8_t sent[] = {HCI_EVENT_TRANSPORT_PACKET_SENT, 0}; + receive_packet(HCI_EVENT_PACKET, sent, sizeof(sent)); +} noreturn void btstack_assert_failed(const char *file, uint16_t line){ fprintf(stderr, "BTstack assertion: %s:%u\n", file, line); @@ -60,8 +68,13 @@ static int can_send(uint8_t packet_type){ } static int send_packet(uint8_t type, uint8_t *packet, int size){ - assert(type == HCI_COMMAND_DATA_PACKET); + assert(type == HCI_COMMAND_DATA_PACKET || type == HCI_ACL_DATA_PACKET); assert(wire_count < 32 && size <= 64); + // CYW43 writes its header before the packet and reads word-rounded data. + assert(((uintptr_t)packet & 3u) == 0); + memset(packet - HCI_OUTGOING_PRE_BUFFER_SIZE, 0xa5, HCI_OUTGOING_PRE_BUFFER_SIZE); + wire_type[wire_count] = type; + last_transport_packet = packet; memcpy(wire[wire_count], packet, (size_t) size); wire_size[wire_count++] = size; if (during_send != NULL){ @@ -69,6 +82,7 @@ static int send_packet(uint8_t type, uint8_t *packet, int size){ during_send = NULL; callback(); } + if (complete_inline) transport_sent(); return 0; } @@ -96,6 +110,7 @@ static void working(void){ // Fixture bypasses controller initialization, leaving the production receive/run paths intact. hci_stack->state = HCI_STATE_WORKING; hci_stack->gap_tasks_classic = 0; + hci_stack->le_scanning_param_update = false; hci_stack->num_cmd_packets = 1; hci_register_acl_packet_handler(on_acl); hci_register_sco_packet_handler(on_sco); @@ -106,6 +121,8 @@ static void begin(uint32_t now, bool asynchronous){ transport_ready = true; transport.can_send_packet_now = asynchronous ? can_send : NULL; during_send = NULL; + complete_inline = false; + last_transport_packet = NULL; wire_count = acl_delivered = sco_delivered = 0; btstack_run_loop_init(&run_loop); btstack_memory_init(); @@ -310,6 +327,125 @@ static void test_lifecycle(void){ assert(wire_count == after_power_transition); finish(); } + +#ifdef SWITCH_PICO_HCI_CREDIT_BUFFER +static void test_credits_bypass_fragmented_acl(unsigned completion_mode){ + begin(100, completion_mode != 0); + complete_inline = completion_mode == 2; + add_connection(0x41, BD_ADDR_TYPE_ACL); + add_connection(0x42, BD_ADDR_TYPE_LE_PUBLIC); + hci_stack->acl_packets_total_num = 1; + hci_stack->acl_data_packet_length = 8; + hci_reserve_packet_buffer(); + uint8_t *packet = hci_get_outgoing_packet_buffer(); + little_endian_store_16(packet, 0, 0x2041); + little_endian_store_16(packet, 2, 16); + for (unsigned i = 0; i < 16; ++i) packet[4 + i] = (uint8_t)(0x80 + i); + assert(hci_send_acl_packet_buffer(20) == ERROR_CODE_SUCCESS); + assert(wire_count == 1 && wire_type[0] == HCI_ACL_DATA_PACKET); + + if (completion_mode == 1) transport_ready = false; + acl(0x42); + acl(0x42); + if (completion_mode == 1){ + advance(2); + assert(wire_count == 1); + transport_ready = true; + transport_sent(); + } + assert(wire_count == 2 && wire_type[1] == HCI_COMMAND_DATA_PACKET); + expect_credits(1, 0x42, 2); + if (completion_mode == 1) transport_sent(); + // A credit completion must not release the still-prepared ACL continuation. + assert(!hci_can_send_command_packet_now()); + + uint8_t completed[] = {HCI_EVENT_NUMBER_OF_COMPLETED_PACKETS, 5, 1, 0x41, 0, 1, 0}; + receive_packet(HCI_EVENT_PACKET, completed, sizeof(completed)); + assert(wire_count == 3 && wire_type[2] == HCI_ACL_DATA_PACKET); + assert(wire_size[0] == 12 && wire_size[2] == 12); + assert(little_endian_read_16(wire[2], 0) == 0x1041); + for (unsigned i = 0; i < 8; ++i){ + assert(wire[0][4 + i] == 0x80 + i); + assert(wire[2][4 + i] == 0x88 + i); + } + if (completion_mode == 1) transport_sent(); + assert(hci_can_send_command_packet_now()); + finish(); +} + +static void test_credit_buffer_async_ownership(void){ + begin(100, true); + add_connection(0x41, BD_ADDR_TYPE_ACL); + hci_reserve_packet_buffer(); + acl(0x41); + advance(2); + assert(wire_count == 1); + uint8_t *in_flight = last_transport_packet; + uint8_t saved[8]; + memcpy(saved, in_flight, sizeof(saved)); + acl(0x41); + advance(2); + assert(wire_count == 1 && memcmp(saved, in_flight, sizeof(saved)) == 0); + // Even a transport that reports ready must not permit overlapping sends. + assert(!hci_can_send_prepared_acl_packet_now(0x41)); + transport_sent(); + assert(wire_count == 2); + expect_credits(1, 0x41, 1); + transport_sent(); + assert(!hci_can_send_command_packet_now()); + hci_release_packet_buffer(); + assert(hci_can_send_command_packet_now()); + finish(); +} + +static void sleep_during_send(void){ + hci_power_control(HCI_POWER_SLEEP); +} + +static void test_credit_completion_during_sleep(void){ + begin(100, false); + add_connection(0x41, BD_ADDR_TYPE_ACL); + acl(0x41); + during_send = sleep_during_send; + advance(2); + assert(wire_count == 1); + expect_credits(0, 0x41, 1); + // Cancelling batching must not strand ownership of a completed synchronous send. + assert(hci_can_send_command_packet_now()); + finish(); + + begin(100, true); + add_connection(0x41, BD_ADDR_TYPE_ACL); + hci_reserve_packet_buffer(); + acl(0x41); + advance(2); + hci_power_control(HCI_POWER_SLEEP); + transport_sent(); + // A late credit completion after sleep still must not release another packet. + assert(!hci_can_send_command_packet_now()); + hci_release_packet_buffer(); + assert(hci_can_send_command_packet_now()); + finish(); +} + +static void test_credit_buffer_power_cycle(void){ + begin(100, true); + add_connection(0x41, BD_ADDR_TYPE_ACL); + acl(0x41); + advance(2); + assert(wire_count == 1); + // A wedged transport never completes the credit send; shutdown closes it. + transport_ready = false; + hci_power_control(HCI_POWER_OFF); + advance(1001); + transport_ready = true; + hci_power_control(HCI_POWER_ON); + assert(wire_count == 2); + assert(wire_type[1] == HCI_COMMAND_DATA_PACKET); + assert(little_endian_read_16(wire[1], 0) == HCI_OPCODE_HCI_RESET); + finish(); +} +#endif #endif int main(void){ @@ -320,6 +456,12 @@ int main(void){ test_sco_and_malformed_acl(); test_synchronous_callbacks(); test_lifecycle(); +#ifdef SWITCH_PICO_HCI_CREDIT_BUFFER + for (unsigned mode = 0; mode < 3; ++mode) test_credits_bypass_fragmented_acl(mode); + test_credit_buffer_async_ownership(); + test_credit_completion_during_sleep(); + test_credit_buffer_power_cycle(); +#endif #else begin(100, false); add_connection(0x41, BD_ADDR_TYPE_ACL); diff --git a/tests/test_btstack_credit_batch_native.py b/tests/test_btstack_credit_batch_native.py index c47df17..9bdd816 100644 --- a/tests/test_btstack_credit_batch_native.py +++ b/tests/test_btstack_credit_batch_native.py @@ -6,8 +6,14 @@ from pathlib import Path import pytest -@pytest.mark.parametrize("batching", [False, True], ids=["default", "batched"]) -def test_btstack_credit_batch_native(tmp_path: Path, batching: bool) -> None: +@pytest.mark.parametrize( + ("batching", "dedicated"), + [(False, False), (True, False), (True, True)], + ids=["default", "batched", "dedicated"], +) +def test_btstack_credit_batch_native( + tmp_path: Path, batching: bool, dedicated: bool +) -> None: root = Path(__file__).resolve().parents[1] sdk = Path( os.environ.get("PICO_SDK_PATH", root / "build" / "_deps" / "pico_sdk-src") @@ -58,6 +64,7 @@ def test_btstack_credit_batch_native(tmp_path: Path, batching: bool) -> None: "-ffunction-sections", "-fdata-sections", *(["-DSWITCH_PICO_HCI_CREDIT_BATCH=1"] if batching else []), + *(["-DSWITCH_PICO_HCI_CREDIT_BUFFER=1"] if dedicated else []), f"-I{root / 'tests' / 'btstack_credit_batch_native_stubs'}", f"-I{patched_source}", f"-I{source}",