tesla ble 4.0 by yoziru · Pull Request #145 · yoziru/esphome-tesla-ble

yoziru · 2026-01-02T17:50:49Z

move logging to tesla-ble
tesla-ble 4.0.0
fixup: improve logging
update tesla_ble_log_callback to include line number
fix wake_vehicle

Copilot

Pull request overview

This PR upgrades the Tesla BLE integration to use version 4.0.0 of the tesla-ble library and performs a significant architectural refactoring. The changes shift responsibility for message handling, session management, and command queuing from local manager classes to the upstream library, simplifying the codebase by removing approximately 2,500 lines of code across multiple manager classes.

Key changes:

Upgrade tesla-ble library from v3.4.0 to v4.0.0
Replace custom manager classes (SessionManager, CommandManager, MessageHandler, BLEManager, PollingManager) with adapter pattern implementations (BleAdapterImpl, StorageAdapterImpl)
Centralize logging through tesla-ble library callback mechanism
Adjust logging levels from VERBOSE to DEBUG for better visibility
Modify state manager methods to return boolean indicating state changes

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
packages/board.yml	Updates tesla-ble library reference from v3.4.0 to v4.0.0
components/tesla_ble_vehicle/vehicle_state_manager.h	Changes publish_sensor_state methods to return bool indicating state changes
components/tesla_ble_vehicle/vehicle_state_manager.cpp	Updates logging levels, modifies state update logic to conditionally log based on changes, removes rollover-safe time calculation
components/tesla_ble_vehicle/tesla_ble_vehicle.h	Major refactoring replacing manager classes with Vehicle instance and adapter pattern
components/tesla_ble_vehicle/tesla_ble_vehicle.cpp	Rewrites initialization, callback wiring, polling logic, and command execution to use tesla-ble library directly
components/tesla_ble_vehicle/storage_adapter_impl.h	New adapter implementing storage interface for NVS operations with key mapping
components/tesla_ble_vehicle/ble_adapter_impl.h	New adapter implementing BLE interface for vehicle communication
components/tesla_ble_vehicle/adapters.cpp	Implements both BLE and storage adapters with queue management and NVS operations
components/tesla_ble_vehicle/session_manager.*	Removed - functionality moved to tesla-ble library
components/tesla_ble_vehicle/polling_manager.*	Removed - polling logic now inline in main component
components/tesla_ble_vehicle/message_handler.*	Removed - message handling delegated to tesla-ble library
components/tesla_ble_vehicle/command_manager.*	Removed - command execution simplified through library calls
components/tesla_ble_vehicle/ble_manager.*	Removed - BLE operations handled by BleAdapterImpl
components/tesla_ble_vehicle/log.*	Removed - logging now handled by tesla-ble library through callback

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-02T17:53:45Z

+    if (publish_sensor_state(asleep_sensor_, asleep)) {
+        ESP_LOGI(STATE_MANAGER_TAG, "Vehicle sleep state: %s", asleep ? "ASLEEP" : "AWAKE");
    }


The publish_sensor_state methods now return bool to indicate if the state actually changed. However, the method is called and its return value is checked, but the polling manager's update_vehicle_state method is never called anymore (removed in refactoring). This means the polling logic that depends on state changes may not function correctly. For example, the vehicle state change detection that was used to determine polling intervals is no longer being updated.

Copilot · 2026-01-02T17:53:45Z

+    TeslaBLEVehicle* parent_;
+    std::queue<BLETXChunk> write_queue_;
+
+    static const size_t BLOCK_LENGTH = 18; // Safe BLE MTU chunk size


The BLE MTU chunk size has been reduced from 20 bytes to 18 bytes. While this might be a safer choice for compatibility, it should be documented why this change was made, especially since the original code used 20 which is a common BLE MTU chunk size. This could impact performance by requiring more fragmentation of messages.

Suggested change

static const size_t BLOCK_LENGTH = 18; // Safe BLE MTU chunk size

// Note: 20 bytes is a common maximum BLE ATT payload size, but we use 18 bytes here

// for improved compatibility with various central devices and BLE stacks that are

// sensitive to MTU/notification sizes. This value has been empirically validated

// for the Tesla BLE use case and avoids edge‑case fragmentation/interop issues

// seen with some phones when using 20‑byte chunks. Increasing this constant may

// improve throughput but MUST be tested carefully on target devices.

static const size_t BLOCK_LENGTH = 18;

Copilot · 2026-01-02T17:53:46Z

+    if (err == ESP_OK) {
+        write_queue_.pop();
+    } else {
+        ESP_LOGW(ADAPTER_TAG, "BLE write failed: %s", esp_err_to_name(err));
+        // Retry? 
+    }


The error handling for BLE write failures only logs a warning and includes a "Retry?" comment, but doesn't actually implement any retry logic. This means failed writes are silently dropped from the queue. The original BLEManager had retry logic with retry_count tracking. Without proper retry or error recovery, commands could be lost, leading to unreliable vehicle communication.

Copilot · 2026-01-02T17:53:46Z

            ESP_LOGI(TAG, "BLE connection fully established");
+
+            // Now that characteristics are ready and notifications are enabled, trigger connection handling
+            handle_connection_established();


In the removed code, the connection was established and polling was triggered after a delay using set_timeout(100, ...). In the new code, this delay has been removed and handle_connection_established() is called directly in the GATTC event handler. However, the comment "Small delay to ensure state is fully set before triggering polling" suggests this delay was intentional. Removing it could cause race conditions where polling attempts to access BLE handles or state that isn't fully initialized yet.

Suggested change

handle_connection_established();

// Small delay to ensure state is fully set before triggering polling

this->set_timeout(100, [this]() {

this->handle_connection_established();

});

Copilot · 2026-01-02T17:53:46Z

+    // Reset initial poll times
+    last_infotainment_poll_ = 0;
+    last_vcsec_poll_ = 0;



The removed code had logic to reset sensor states and availability on connection loss through state_manager_->set_sensors_available(false) and state_manager_->reset_all_states(). These calls have been removed in the new implementation. This means sensors may continue to show stale data from before the connection was lost, misleading users about the actual vehicle state.

Suggested change

if (state_manager_) {

// Mark sensors as unavailable and clear their states to avoid stale data

state_manager_->set_sensors_available(false);

state_manager_->reset_all_states();

}

Copilot · 2026-01-02T17:53:46Z

    uint32_t now = millis();
    // Rollover-safe time calculation
-    uint32_t time_since_command = Utils::time_since(now, last_command_time_);
+    uint32_t time_since_command = (now - last_command_time_);


The time calculation has been changed from the rollover-safe utility function Utils::time_since() to a simple subtraction (now - last_command_time_). This makes the code vulnerable to timer rollover issues. When millis() rolls over after approximately 49 days, the subtraction will produce incorrect results and potentially very large time values, causing commands to be incorrectly delayed or not delayed when they should be.

Copilot · 2026-01-02T17:53:47Z

+    if (state_manager_->is_asleep()) {
+         // If asleep, we might want to poll less frequently or not at all unless forced via timeout/logic.
+         // Original logic used sleep timeout. For simplicity here:
+         infotainment_interval = infotainment_sleep_timeout_; 
+    }
+
+    // Check for "active" state to use faster polling
+    if (!state_manager_->is_asleep()) {
+        if (state_manager_->is_charging() || state_manager_->is_user_present() || state_manager_->is_unlocked()) {
+            infotainment_interval = infotainment_poll_interval_active_;
+            ESP_LOGD(TAG, "Vehicle is active, using active polling interval (%dms)", infotainment_interval);
+        }
+    }


The conditional logic here doesn't make sense. The code checks if (state_manager_->is_asleep()) to decide whether to use fast polling, but then immediately checks if (!state_manager_->is_asleep()) to determine if the vehicle is active. If the vehicle is asleep, the interval is set to infotainment_sleep_timeout_, but then the second condition will never be true. This creates unreachable code and potentially incorrect polling behavior.

Copilot · 2026-01-02T17:53:47Z

+            // conn_id is managed by BLEClient
            this->read_handle_ = 0;
            this->write_handle_ = 0;
            this->node_state = espbt::ClientState::DISCONNECTING;


The removed code stored the connection ID in this->handle_ and cleared it on disconnect. The new code only has a comment saying "conn_id is managed by BLEClient" but doesn't actually clear or manage the connection ID properly. While the read_handle_ and write_handle_ are cleared, connection state management has been weakened.

Suggested change

this->node_state = espbt::ClientState::DISCONNECTING;

handle_connection_lost();

Copilot · 2026-01-02T17:53:47Z

+const char* StorageAdapterImpl::map_key(const std::string& key) {
+    if (key == "session_vcsec") return "tk_vcsec";
+    if (key == "session_infotainment") return "tk_infotainment";
+    if (key == "private_key") return "private_key"; // Unchanged
+    return nullptr;
+}


The implementation returns nullptr for unknown keys, but this is not checked by the callers in the load, save, and remove methods - they simply return false. However, an unknown key might indicate a programming error. Consider logging a warning when an unknown key is encountered to aid debugging, as the silent failure makes it hard to detect when the library's key naming conventions change.

yoziru added 5 commits January 1, 2026 15:35

move logging to tesla-ble

bcf56d7

tesla-ble 4.0.0

b952e8e

fixup: improve logging

3403393

update tesla_ble_log_callback to include line number

2f270c7

fix wake_vehicle

327dbe4

Copilot AI review requested due to automatic review settings January 2, 2026 17:50

yoziru enabled auto-merge (squash) January 2, 2026 17:50

Copilot started reviewing on behalf of yoziru January 2, 2026 17:51 View session

Copilot AI reviewed Jan 2, 2026

View reviewed changes

yoziru merged commit 7c4407d into main Jan 2, 2026
9 checks passed

yoziru deleted the tesla-ble-4.0 branch January 2, 2026 17:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

tesla ble 4.0#145

tesla ble 4.0#145
yoziru merged 5 commits into
mainfrom
tesla-ble-4.0

yoziru commented Jan 2, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 2, 2026

Uh oh!

Copilot AI Jan 2, 2026

Uh oh!

Copilot AI Jan 2, 2026

Uh oh!

Copilot AI Jan 2, 2026

Uh oh!

Copilot AI Jan 2, 2026

Uh oh!

Copilot AI Jan 2, 2026

Uh oh!

Copilot AI Jan 2, 2026

Uh oh!

Copilot AI Jan 2, 2026

Uh oh!

Copilot AI Jan 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-    static const size_t BLOCK_LENGTH = 18; // Safe BLE MTU chunk size
+    // Note: 20 bytes is a common maximum BLE ATT payload size, but we use 18 bytes here
+    // for improved compatibility with various central devices and BLE stacks that are
+    // sensitive to MTU/notification sizes. This value has been empirically validated
+    // for the Tesla BLE use case and avoids edge‑case fragmentation/interop issues
+    // seen with some phones when using 20‑byte chunks. Increasing this constant may
+    // improve throughput but MUST be tested carefully on target devices.
+    static const size_t BLOCK_LENGTH = 18;

-            handle_connection_established();
+            // Small delay to ensure state is fully set before triggering polling
+            this->set_timeout(100, [this]() {
+                this->handle_connection_established();
+            });

+    if (state_manager_) {
+        // Mark sensors as unavailable and clear their states to avoid stale data
+        state_manager_->set_sensors_available(false);
+        state_manager_->reset_all_states();
+    }

	this->node_state = espbt::ClientState::DISCONNECTING;
	handle_connection_lost();

Uh oh!

Conversation

yoziru commented Jan 2, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants