diff --git a/rust/src/embassy/time_driver.rs b/rust/src/embassy/time_driver.rs index c0b0e46..3f0acf8 100644 --- a/rust/src/embassy/time_driver.rs +++ b/rust/src/embassy/time_driver.rs @@ -57,10 +57,25 @@ static LAST_CLOCK_TIME: AtomicU32 = AtomicU32::new(0); /// to safely handle concurrent access without locks in the common case. /// /// # Algorithm -/// 1. Read current hardware time and previously seen time -/// 2. If current time < last time, we may have an overflow -/// 3. Use a critical section to safely update the upper 32 bits if overflow confirmed +/// 1. Inside a critical section (interrupts disabled), read the current hardware time +/// and the previously seen time atomically together +/// 2. If current time < last time, the 32-bit hardware counter has overflowed - increment +/// the upper 32 bits to extend the counter +/// 3. Update the last seen time /// 4. Return a combined 64-bit timestamp (upper 32 bits | lower 32 bits) +/// +/// # Why clock_time() must be called inside the critical section +/// +/// Previously, clock_time() was called BEFORE entering the critical section. This created +/// a race condition: if a hardware interrupt fired between reading clock_time() and reading +/// LAST_CLOCK_TIME, and that interrupt handler itself called clock_time64() (e.g. to step +/// a light transition), it would update LAST_CLOCK_TIME to a newer value. When the +/// interrupted code resumed, LAST_CLOCK_TIME was now genuinely larger than the stale +/// current_time, so the code falsely detected a 32-bit overflow and incremented the upper +/// bits. At 16-24 MHz this adds ~180-268 seconds to Instant::now() instantaneously, which +/// blew past any in-progress transition timestamp and caused the light to snap to its final +/// state. By reading clock_time() inside the critical section, interrupts are physically +/// incapable of wedging themselves between the hardware read and the comparison. #[cfg_attr(test, mry::mry)] pub fn clock_time64() -> u64 { // When not testing, these static variables are defined here @@ -70,38 +85,28 @@ pub fn clock_time64() -> u64 { #[cfg(not(test))] static LAST_CLOCK_TIME: AtomicU32 = AtomicU32::new(0); - // Get current hardware time - let current_time = clock_time(); - let last_time = LAST_CLOCK_TIME.load(Ordering::Relaxed); - - // Only enter critical section if we suspect an overflow - // (current time is less than last seen time) - if current_time < last_time { - critical_section::with(|_| { - // Re-check within critical section to avoid race conditions - // This prevents multiple threads from incrementing the upper bits - let last_time_cs = LAST_CLOCK_TIME.load(Ordering::Relaxed); - if current_time < last_time_cs { - // Overflow confirmed - increment upper bits - // Use load-modify-store instead of fetch_add since fetch_add might not be supported - // on all platforms or with all atomics implementations - let upper = CLOCK_TIME_UPPER.load(Ordering::Relaxed); - CLOCK_TIME_UPPER.store(upper + 1, Ordering::Relaxed); - } - - // Update last time seen within the critical section - LAST_CLOCK_TIME.store(current_time, Ordering::Relaxed); - }); - } else { - // Normal case (no overflow) - just update the last seen time - // This fast path avoids the critical section in most calls + critical_section::with(|_| { + // Read the hardware time inside the critical section so that no interrupt can update + // LAST_CLOCK_TIME between this read and the comparison below. + let current_time = clock_time(); + let last_time = LAST_CLOCK_TIME.load(Ordering::Relaxed); + + if current_time < last_time { + // Overflow confirmed - increment upper bits. + // Use load-modify-store instead of fetch_add since fetch_add might not be supported + // on all platforms or with all atomics implementations. + let upper = CLOCK_TIME_UPPER.load(Ordering::Relaxed); + CLOCK_TIME_UPPER.store(upper + 1, Ordering::Relaxed); + } + + // Always update the last seen time so subsequent calls can detect the next overflow. LAST_CLOCK_TIME.store(current_time, Ordering::Relaxed); - } - // Combine upper and lower bits to form the 64-bit timestamp - // Upper 32 bits track number of overflows - // Lower 32 bits are the current hardware timer value - (CLOCK_TIME_UPPER.load(Ordering::Relaxed) as u64) << 32 | current_time as u64 + // Combine upper and lower bits to form the 64-bit timestamp. + // Upper 32 bits track the number of overflows. + // Lower 32 bits are the current hardware timer value. + (CLOCK_TIME_UPPER.load(Ordering::Relaxed) as u64) << 32 | current_time as u64 + }) } #[cfg(test)] diff --git a/rust/src/main_light.rs b/rust/src/main_light.rs index 4dd4796..21978ae 100644 --- a/rust/src/main_light.rs +++ b/rust/src/main_light.rs @@ -272,7 +272,7 @@ pub fn rf_link_response_callback(ppp: &mut PacketAttValue, p_req: &PacketAttValu let group_address = GROUP_ADDRESS.lock(); let mut idx = 0; - match p_req.val[15] { + match ppp.val[15] { GET_STATUS => { ppp.val[0] = LGT_CMD_LIGHT_STATUS | 0xc0; @@ -1103,9 +1103,9 @@ mod tests { let mut request = create_test_packet_att_value(); request.src = [0x56, 0x78]; - request.val[15] = GET_GROUP1; let mut response = create_test_packet_att_value(); + response.val[15] = GET_GROUP1; // Execute let result = rf_link_response_callback(&mut response, &request); @@ -1134,9 +1134,9 @@ mod tests { let mut request = create_test_packet_att_value(); request.src = [0x56, 0x78]; - request.val[15] = GET_GROUP2; let mut response = create_test_packet_att_value(); + response.val[15] = GET_GROUP2; // Execute let result = rf_link_response_callback(&mut response, &request); @@ -1169,9 +1169,9 @@ mod tests { let mut request = create_test_packet_att_value(); request.src = [0x56, 0x78]; - request.val[15] = GET_GROUP3; let mut response = create_test_packet_att_value(); + response.val[15] = GET_GROUP3; // Execute let result = rf_link_response_callback(&mut response, &request); @@ -1194,9 +1194,9 @@ mod tests { let mut request = create_test_packet_att_value(); request.src = [0x56, 0x78]; - request.val[15] = GET_DEV_ADDR; let mut response = create_test_packet_att_value(); + response.val[15] = GET_DEV_ADDR; // Execute let result = rf_link_response_callback(&mut response, &request); @@ -1216,9 +1216,9 @@ mod tests { let mut request = create_test_packet_att_value(); request.src = [0x56, 0x78]; - request.val[15] = GET_USER_NOTIFY; let mut response = create_test_packet_att_value(); + response.val[15] = GET_USER_NOTIFY; // Execute let result = rf_link_response_callback(&mut response, &request); @@ -1248,9 +1248,9 @@ mod tests { let mut request = create_test_packet_att_value(); request.src = [0x56, 0x78]; - request.val[15] = CMD_START_OTA; let mut response = create_test_packet_att_value(); + response.val[15] = CMD_START_OTA; // Execute let result = rf_link_response_callback(&mut response, &request); @@ -1284,9 +1284,9 @@ mod tests { let mut request = create_test_packet_att_value(); request.src = [0xAA, 0xBB]; - request.val[15] = CMD_OTA_DATA; let mut response = create_test_packet_att_value(); + response.val[15] = CMD_OTA_DATA; // Execute let result = rf_link_response_callback(&mut response, &request); @@ -1317,9 +1317,9 @@ mod tests { let mut request = create_test_packet_att_value(); request.src = [0xCC, 0xDD]; - request.val[15] = CMD_END_OTA; let mut response = create_test_packet_att_value(); + response.val[15] = CMD_END_OTA; // Execute let result = rf_link_response_callback(&mut response, &request); @@ -1342,9 +1342,9 @@ mod tests { DEVICE_ADDRESS.set(0x1234); let mut request = create_test_packet_att_value(); - request.val[15] = 0xFF; // Invalid command let mut response = create_test_packet_att_value(); + response.val[15] = 0xFF; // Invalid command // Execute let result = rf_link_response_callback(&mut response, &request); diff --git a/rust/src/sdk/ble_app/light_ll/packet_processing.rs b/rust/src/sdk/ble_app/light_ll/packet_processing.rs index ad0fe4e..4019606 100644 --- a/rust/src/sdk/ble_app/light_ll/packet_processing.rs +++ b/rust/src/sdk/ble_app/light_ll/packet_processing.rs @@ -580,11 +580,11 @@ pub fn rf_link_slave_add_status(packet: &Packet) { // Copy the source address (2 bytes) let src_bytes = packet.mesh().src_adr.to_le_bytes(); st_ptr.att_data_mut().dat[5..7].copy_from_slice(&src_bytes); + // Copy the operation code (1 byte) — op precedes vendor_id in the mesh packet layout + st_ptr.att_data_mut().dat[7] = packet.mesh().op; // Copy the vendor ID (2 bytes) let vendor_bytes = packet.mesh().vendor_id.to_le_bytes(); - st_ptr.att_data_mut().dat[7..9].copy_from_slice(&vendor_bytes); - // Copy the operation code (1 byte) - st_ptr.att_data_mut().dat[9] = packet.mesh().op; + st_ptr.att_data_mut().dat[8..10].copy_from_slice(&vendor_bytes); // Copy the parameters (10 bytes) st_ptr.att_data_mut().dat[10..20].copy_from_slice(&packet.mesh().par); @@ -2111,8 +2111,70 @@ mod tests { ); } - // ================================================================================ - // Tests for is_add_packet_buf_ready function + /// Tests that the assembled BLE notification packet has op at dat[7] and + /// vendor_id at dat[8..10], matching the mesh packet memory layout and the + /// format expected by the Python client's decrypt_notification parser. + #[test] + #[mry::lock()] + fn test_rf_link_slave_add_status_packet_layout() { + reset_test_state(); + SLAVE_READ_STATUS_BUSY.set(LGT_CMD_LIGHT_STATUS); // allow notify req mask + + let mut packet = create_test_packet( + LGT_CMD_LIGHT_STATUS | 0xc0, + [0xAA, 0xBB, 0xCC], + 0x001F, // src_adr = 31 + 0x0000, // dst_adr + ); + // Set known par values so we can verify them in the output + packet.att_cmd_mut().value.val[3] = 0x11; // par[0] = CW lo + packet.att_cmd_mut().value.val[4] = 0x22; // par[1] = CW hi + + rf_link_slave_add_status(&packet); + + assert_eq!( + DEVICE_STATUS_BUFFER_WRITE_POINTER.get(), + 1, + "Write pointer should advance" + ); + + let buf = BUFF_RESPONSE.lock(); + let st = &buf[0]; + + // dat[0..3] = sno + assert_eq!( + &st.att_data().dat[0..3], + &[0xAA, 0xBB, 0xCC], + "sno mismatch" + ); + // dat[3..5] = src_adr (overwritten from dst then from src at the end) + assert_eq!(st.att_data().dat[3], 0x1F, "src_adr low byte"); + assert_eq!(st.att_data().dat[4], 0x00, "src_adr high byte"); + // dat[5..7] = src_adr (original src copy) + assert_eq!(st.att_data().dat[5], 0x1F, "dat[5] src low"); + assert_eq!(st.att_data().dat[6], 0x00, "dat[6] src high"); + // dat[7] = op — MUST come before vendor_id so Python parser reads it at index 0 + // of the decrypted payload (after sno+src+mic are stripped) + assert_eq!( + st.att_data().dat[7], + LGT_CMD_LIGHT_STATUS | 0xc0, + "dat[7] must be opcode (not vendor_id)" + ); + // dat[8..10] = vendor_id (little-endian) + assert_eq!( + st.att_data().dat[8], + (VENDOR_ID & 0xFF) as u8, + "vendor_id lo" + ); + assert_eq!( + st.att_data().dat[9], + ((VENDOR_ID >> 8) & 0xFF) as u8, + "vendor_id hi" + ); + // dat[10..12] = first two par bytes + assert_eq!(st.att_data().dat[10], 0x11, "par[0] = CW lo"); + assert_eq!(st.att_data().dat[11], 0x22, "par[1] = CW hi"); + } // ================================================================================ /// Tests transmission buffer ready status with empty buffer. diff --git a/rust/src/version.rs b/rust/src/version.rs index 82be987..069534d 100644 --- a/rust/src/version.rs +++ b/rust/src/version.rs @@ -1 +1 @@ -pub static BUILD_VERSION: u32 = 3515; +pub static BUILD_VERSION: u32 = 3518; diff --git a/sdk/version.in b/sdk/version.in index acf829c..d740839 100644 --- a/sdk/version.in +++ b/sdk/version.in @@ -1,2 +1,2 @@ -.equ BUILD_VERSION,3515 +.equ BUILD_VERSION,3517 .equ XTAL_16MHZ,0 diff --git a/utilities/meshutils/mesh_add.py b/utilities/meshutils/mesh_add.py index 8c4bb00..6241fdc 100644 --- a/utilities/meshutils/mesh_add.py +++ b/utilities/meshutils/mesh_add.py @@ -107,134 +107,138 @@ async def main(): return print(f"Found device {device.address}, connecting...") - async with BleakClient(device.address) as client: - # ----- PHASE 1: AUTHENTICATION AND SESSION KEY ESTABLISHMENT ----- - print("Authenticating with device...") - session_key = await authenticate(client, DEFAULT_MESH_NAME, DEFAULT_MESH_PASSWORD) - print("Authentication successful, session established") - - # ----- PHASE 2: DEVICE CONFIGURATION FOR MESH ----- - print("Configuring device for mesh network...") - - # Get mesh address from command line arguments - mesh_address = args.mesh_address - print(f"Assigning mesh address: {mesh_address}") - - # Get device MAC address and reverse for protocol compatibility - _, mac_bytes = parse_mac_address(device.address) - - print("Setting device mesh address...") - # Send command to set mesh address (opcode 0xE0) - action = BaseCommandAction( - mac_address=mac_bytes, - opcode=0xe0, # Command to set mesh address - params=[mesh_address & 0xff, (mesh_address >> 8) & 0xff], # Little-endian address - mesh_address=0, # Direct to device (not through mesh) - session_key=session_key, - vendor_id=0x0211, # Telink vendor ID (LE: [0x11, 0x02] on wire) - no_response=True - ).build_command_action() - - await action.encode_and_send(client) - - # Allow time for the device to process the mesh address - await sleep(4) - - # ----- PHASE 3: PROVISIONING MESH PARAMETERS ----- - print("Provisioning mesh parameters...") - - # Prepare and encrypt mesh name - name = mesh_name.encode() - # Pad name to 16 bytes - name = bytearray(name) + bytearray([0] * (16 - len(name))) - # Encrypt name with session key - name = encrypt_data(session_key, name) - # Reverse bytes for device compatibility - name.reverse() - - # Prepare and encrypt mesh password - pwd = mesh_password.encode() - # Pad password to 16 bytes - pwd = bytearray(pwd) + bytearray([0] * (16 - len(pwd))) - # Encrypt password with session key - pwd = encrypt_data(session_key, pwd) - # Reverse bytes for device compatibility - pwd.reverse() - - # Prepare and encrypt long term key (LTK) - # LTK validation and user confirmation was done earlier - - # Encrypt the LTK with the session key - ltk = encrypt_data(session_key, ltk_bytes) - # Reverse bytes for device compatibility - ltk.reverse() - - # Add opcodes to the start of each parameter - name = bytearray([PAIR_OP_SET_MESH_NAME]) + name - pwd = bytearray([PAIR_OP_SET_MESH_PASSWORD]) + pwd - # Add mesh flag to LTK to indicate this is for mesh communication - ltk = bytearray([PAIR_OP_SET_MESH_LTK]) + ltk + bytearray([MESH_FLAG]) - - # Create commands for each parameter - name_command = Command(name, pair_characteristic_uuid, client) - pwd_command = Command(pwd, pair_characteristic_uuid, client) - ltk_command = Command(ltk, pair_characteristic_uuid, client) - - # Send mesh parameters in sequence - print("Sending mesh name...") - await name_command.write() - await sleep(0.2) # Allow device time to process - - print("Sending mesh password...") - await pwd_command.write() - await sleep(0.2) # Allow device time to process - - print("Sending mesh LTK...") - await ltk_command.write() - await sleep(0.2) # Allow device time to process - - # ----- PHASE 4: VERIFY SUCCESSFUL PAIRING ----- - # Read the device state to verify successful pairing - # Give the device a bit more time to complete state transitions - await sleep(2) - result = await client.read_gatt_char(pair_characteristic_uuid) - - # Check if device is in the expected state (pairing complete) - # If it's in MeshPairEffect (0x07), try waiting a bit more - if result[0] == PAIR_STATE_MESH_EFFECT: # MeshPairEffect - almost complete - print("Device in MeshPairEffect state, waiting for final transition...") - await sleep(1.0) + try: + async with BleakClient(device.address) as client: + # ----- PHASE 1: AUTHENTICATION AND SESSION KEY ESTABLISHMENT ----- + print("Authenticating with device...") + session_key = await authenticate(client, DEFAULT_MESH_NAME, DEFAULT_MESH_PASSWORD) + print("Authentication successful, session established") + + # ----- PHASE 2: DEVICE CONFIGURATION FOR MESH ----- + print("Configuring device for mesh network...") + + # Get mesh address from command line arguments + mesh_address = args.mesh_address + print(f"Assigning mesh address: {mesh_address}") + + # Get device MAC address and reverse for protocol compatibility + _, mac_bytes = parse_mac_address(device.address) + + print("Setting device mesh address...") + # Send command to set mesh address (opcode 0xE0) + action = BaseCommandAction( + mac_address=mac_bytes, + opcode=0xe0, # Command to set mesh address + params=[mesh_address & 0xff, (mesh_address >> 8) & 0xff], # Little-endian address + mesh_address=0, # Direct to device (not through mesh) + session_key=session_key, + vendor_id=0x0211, # Telink vendor ID (LE: [0x11, 0x02] on wire) + no_response=True + ).build_command_action() + + await action.encode_and_send(client) + + # Allow time for the device to process the mesh address + await sleep(4) + + # ----- PHASE 3: PROVISIONING MESH PARAMETERS ----- + print("Provisioning mesh parameters...") + + # Prepare and encrypt mesh name + name = mesh_name.encode() + # Pad name to 16 bytes + name = bytearray(name) + bytearray([0] * (16 - len(name))) + # Encrypt name with session key + name = encrypt_data(session_key, name) + # Reverse bytes for device compatibility + name.reverse() + + # Prepare and encrypt mesh password + pwd = mesh_password.encode() + # Pad password to 16 bytes + pwd = bytearray(pwd) + bytearray([0] * (16 - len(pwd))) + # Encrypt password with session key + pwd = encrypt_data(session_key, pwd) + # Reverse bytes for device compatibility + pwd.reverse() + + # Prepare and encrypt long term key (LTK) + # LTK validation and user confirmation was done earlier + + # Encrypt the LTK with the session key + ltk = encrypt_data(session_key, ltk_bytes) + # Reverse bytes for device compatibility + ltk.reverse() + + # Add opcodes to the start of each parameter + name = bytearray([PAIR_OP_SET_MESH_NAME]) + name + pwd = bytearray([PAIR_OP_SET_MESH_PASSWORD]) + pwd + # Add mesh flag to LTK to indicate this is for mesh communication + ltk = bytearray([PAIR_OP_SET_MESH_LTK]) + ltk + bytearray([MESH_FLAG]) + + # Create commands for each parameter + name_command = Command(name, pair_characteristic_uuid, client) + pwd_command = Command(pwd, pair_characteristic_uuid, client) + ltk_command = Command(ltk, pair_characteristic_uuid, client) + + # Send mesh parameters in sequence + print("Sending mesh name...") + await name_command.write() + await sleep(0.2) # Allow device time to process + + print("Sending mesh password...") + await pwd_command.write() + await sleep(0.2) # Allow device time to process + + print("Sending mesh LTK...") + await ltk_command.write() + await sleep(0.2) # Allow device time to process + + # ----- PHASE 4: VERIFY SUCCESSFUL PAIRING ----- + # Read the device state to verify successful pairing + # Give the device a bit more time to complete state transitions + await sleep(2) result = await client.read_gatt_char(pair_characteristic_uuid) - - if result[0] != PAIR_STATE_PAIRING_COMPLETE: - # Accept MeshPairEffect (0x07) as success too, since it's very close - if result[0] == PAIR_STATE_MESH_EFFECT: - print(f"Device in MeshPairEffect state (0x{PAIR_STATE_MESH_EFFECT:02x}) - pairing likely successful") + + # Check if device is in the expected state (pairing complete) + # If it's in MeshPairEffect (0x07), try waiting a bit more + if result[0] == PAIR_STATE_MESH_EFFECT: # MeshPairEffect - almost complete + print("Device in MeshPairEffect state, waiting for final transition...") + await sleep(1.0) + result = await client.read_gatt_char(pair_characteristic_uuid) + + if result[0] != PAIR_STATE_PAIRING_COMPLETE: + # Accept MeshPairEffect (0x07) as success too, since it's very close + if result[0] == PAIR_STATE_MESH_EFFECT: + print(f"Device in MeshPairEffect state (0x{PAIR_STATE_MESH_EFFECT:02x}) - pairing likely successful") + else: + print(f"Light could not be added to mesh. Unexpected state: 0x{result[0]:02x}") + print(f"Expected state: 0x{PAIR_STATE_PAIRING_COMPLETE:02x} (PAIR_STATE_PAIRING_COMPLETE)") + return + + print(f"Light with MAC ({device.address}) successfully added to mesh network") + print(f"Mesh Name: {mesh_name}") + print(f"Mesh Address: {mesh_address}") + if using_default_ltk: + print("Using default LTK") else: - print(f"Light could not be added to mesh. Unexpected state: 0x{result[0]:02x}") - print(f"Expected state: 0x{PAIR_STATE_PAIRING_COMPLETE:02x} (PAIR_STATE_PAIRING_COMPLETE)") - return - - print(f"Light with MAC ({device.address}) successfully added to mesh network") - print(f"Mesh Name: {mesh_name}") - print(f"Mesh Address: {mesh_address}") - if using_default_ltk: - print("Using default LTK") - else: - print("Using custom LTK") - - # Set up notification handler for any device messages - def on_message(*args, **kwargs): - print("Message received:", args, kwargs) - - # Enable notifications - print("Enabling notifications...") - await client.write_gatt_char(notify_characteristic_uuid, [1]) - await client.start_notify(notify_characteristic_uuid, on_message) - - # Keep connection open to observe any messages - print("Monitoring device for 120 seconds...") - await asyncio.sleep(120) + print("Using custom LTK") + + # Set up notification handler for any device messages + def on_message(*args, **kwargs): + print("Message received:", args, kwargs) + + # Enable notifications + print("Enabling notifications...") + await client.write_gatt_char(notify_characteristic_uuid, [1]) + await client.start_notify(notify_characteristic_uuid, on_message) + + # Keep connection open to observe any messages + print("Monitoring device for 10 seconds...") + await asyncio.sleep(10) + except EOFError: + # Suppress EOFError which can happen during disconnection if the device resets + pass # Run the main function when script is executed diff --git a/utilities/meshutils/mesh_common.py b/utilities/meshutils/mesh_common.py index 9dbcc8f..986a717 100644 --- a/utilities/meshutils/mesh_common.py +++ b/utilities/meshutils/mesh_common.py @@ -691,7 +691,8 @@ def decrypt_notification(data, session_key, mac_bytes_fwd): mac_bytes_fwd: Device MAC address in display order (MSB first). Returns: - Decrypted payload as bytearray, or None if MIC verification fails. + Decrypted payload as bytearray with layout (op[1] | vendor_id[2] | params[10]), + or None if MIC verification fails. """ if len(data) < 8: return None