diff --git a/CHANGELOG.md b/CHANGELOG.md index 44d8dc5..423f40e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ All notable changes are listed here. `Unreleased` is what `main` and the Docker ### Breaking - `/gen_keys` and `/remove_keys` accept `POST` only. `GET` now returns `405`. The dashboard already uses `POST`; scripts or bookmarks that call these endpoints with `GET` must switch. ([#170](https://github.com/wimaha/TeslaBleHttpProxy/pull/170)) +- Possibly breaking for integrations: requests that fail are now always reported as failures (`result: false`, HTTP 503). Before, some of them, for example `set_charging_amps` without a body, returned `result: true` without doing anything. Commands sent without `wait=true` that cannot be queued (queue full, key change) now return 503 instead of "will be processed shortly". `body_controller_state` waits in the command queue and answers with an error after 15 seconds if the proxy is busy, for example retrying while the car is out of range. ([#174](https://github.com/wimaha/TeslaBleHttpProxy/pull/174)) ### Security @@ -17,14 +18,24 @@ All notable changes are listed here. `Unreleased` is what `main` and the Docker - `trustedOrigins` environment variable for reverse proxies over plain HTTP that rewrite the `Host` header, and for web pages on other origins that call the API. See [environment variables](docs/environment_variables.md#trustedorigins). ([#170](https://github.com/wimaha/TeslaBleHttpProxy/pull/170)) - Docker image `:dev`, built from `main` on every push. ([#171](https://github.com/wimaha/TeslaBleHttpProxy/pull/171)) +- `releaseAdapterWhenIdle` environment variable: hands the Bluetooth adapter back to the system between connections, so other Bluetooth services on the same host can use it. Off by default. Fixes [#158](https://github.com/wimaha/TeslaBleHttpProxy/issues/158), based on [#161](https://github.com/wimaha/TeslaBleHttpProxy/pull/161) by @AloisKlingler. +- `connectionTimeout` environment variable: how long a connection stays open after a command (default 29 seconds as before). Based on [#161](https://github.com/wimaha/TeslaBleHttpProxy/pull/161) by @AloisKlingler. - README examples for `set_charge_limit` and `set_sentry_mode`. ([#169](https://github.com/wimaha/TeslaBleHttpProxy/pull/169)) ### Fixed +- `set_charging_amps`, `set_charge_limit` and `set_sentry_mode` with a missing or invalid body answer with an error before the car is contacted. Before, they could report success without doing anything. Other errors that are not retried are reported too instead of coming back as success. +- `body_controller_state` goes through the command queue: it no longer opens a second Bluetooth connection next to a running command, and it is answered right away on an open connection. +- Changing or generating keys in the dashboard stops the previous command loop instead of leaving it running next to the new one. Requests during the change get a clear error instead of hanging. - A client that disconnects while a response is written no longer stops the proxy. ([#170](https://github.com/wimaha/TeslaBleHttpProxy/pull/170)) ### Changed +- `body_controller_state` waits in the command queue like every other request. If the proxy is busy with another command or retrying a connection, it answers with an error after at most 15 seconds instead of opening a second connection. A caller that gave up is not sent to the vehicle later. +- Errors are reported more strictly: requests that failed without being retried (for example a missing body) now return `result: false` (HTTP 503), where some used to return `result: true`. +- When the command queue is full (50 waiting requests, the vehicle has been unreachable for a while), new requests are answered with an error right away instead of waiting. +- Sending a key to the vehicle from the dashboard closes an idle connection of the command loop first and waits at most 30 seconds for a busy one. +- Updated BLE library ([wimaha/ble_BleConnectFix#1](https://github.com/wimaha/ble_BleConnectFix/pull/1)): when the proxy closes the adapter, it is brought back up, so BlueZ sees it again. Needed for `releaseAdapterWhenIdle`. - Building from source requires Go 1.26 or newer. ([#172](https://github.com/wimaha/TeslaBleHttpProxy/pull/172)) - Tests run against a simulated vehicle, so commands, connection handling and the HTTP API are covered without a car. ([#169](https://github.com/wimaha/TeslaBleHttpProxy/pull/169)) diff --git a/README.md b/README.md index 57edeea..b43b164 100644 --- a/README.md +++ b/README.md @@ -239,6 +239,10 @@ This is recommended if you want to receive data frequently, since it will reduce The body controller state is fetched from the vehicle and returnes the state of the body controller. The request does not wake up the vehicle. The following information is returned: +*(unreleased)* + +The request goes through the same queue as commands, so there is only ever one Bluetooth connection to the vehicle. If a connection is already open, it is answered on it right away. If the proxy is busy with another command (for example retrying while the vehicle is out of range), the request waits for at most 15 seconds and then answers with an error. + - `vehicleLockState` - `VEHICLELOCKSTATE_UNLOCKED` - `VEHICLELOCKSTATE_LOCKED` @@ -287,6 +291,12 @@ Due to BLE's power-saving design, Tesla vehicles may terminate connections after This is a Tesla vehicle constraint, not a limitation of TeslaBleHttpProxy. +### Sharing the Bluetooth adapter with other services + +*(unreleased)* + +By default the proxy keeps the Bluetooth adapter for itself while it runs, so other Bluetooth services on the same host (for example a door lock using BlueZ) report "No Bluetooth adapters found". Set [`releaseAdapterWhenIdle=true`](docs/environment_variables.md#releaseadapterwhenidle) to hand the adapter back whenever no connection to the car is open, and a lower [`connectionTimeout`](docs/environment_variables.md#connectiontimeout) such as `10` to free it sooner after a command. + ### Dashboard buttons return 403 Forbidden *(unreleased)* diff --git a/config/config.go b/config/config.go index 095400b..1e2feb2 100644 --- a/config/config.go +++ b/config/config.go @@ -87,12 +87,14 @@ func GetKeyFilesForRole(role string) (string, string) { var Version = "*undefined*" type Config struct { - LogLevel string - HttpListenAddress string - ScanTimeout int // Seconds to scan for BLE devices - CacheMaxAge int // Seconds for HTTP Cache-Control header max-age (used for body controller state responses). If set to 0, cache headers are disabled. - VehicleDataCacheTime int // Seconds to cache VehicleData endpoint responses in memory. Each endpoint is cached separately per VIN. - TrustedOrigins []string // Origins (scheme://host[:port]) allowed to send state-changing requests from a browser, e.g. a reverse proxy that rewrites the Host header. + LogLevel string + HttpListenAddress string + ScanTimeout int // Seconds to scan for BLE devices + CacheMaxAge int // Seconds for HTTP Cache-Control header max-age (used for body controller state responses). If set to 0, cache headers are disabled. + VehicleDataCacheTime int // Seconds to cache VehicleData endpoint responses in memory. Each endpoint is cached separately per VIN. + TrustedOrigins []string // Origins (scheme://host[:port]) allowed to send state-changing requests from a browser, e.g. a reverse proxy that rewrites the Host header. + ConnectionTimeout int // Seconds a BLE connection is kept open after a command for further commands. + ReleaseAdapterWhenIdle bool // Hand the BLE adapter back to the system between connections, so other Bluetooth services can use it. } var AppConfig *Config @@ -146,6 +148,26 @@ func LoadConfig() *Config { } logging.Info("Env:", "vehicleDataCacheTime", vehicleDataCacheTimeInt) + connectionTimeout := os.Getenv("connectionTimeout") + if connectionTimeout == "" { + connectionTimeout = "29" // default value + } + connectionTimeoutInt, err := strconv.Atoi(connectionTimeout) + if err != nil || connectionTimeoutInt <= 0 { + logging.Error("Invalid connectionTimeout value, using default (29)", "value", connectionTimeout, "error", err) + connectionTimeoutInt = 29 + } + logging.Info("Env:", "connectionTimeout", connectionTimeoutInt) + + releaseAdapterWhenIdle := false + if value := os.Getenv("releaseAdapterWhenIdle"); value != "" { + if releaseAdapterWhenIdle, err = strconv.ParseBool(value); err != nil { + logging.Error("Invalid releaseAdapterWhenIdle value, using default (false)", "value", value, "error", err) + releaseAdapterWhenIdle = false + } + } + logging.Info("Env:", "releaseAdapterWhenIdle", releaseAdapterWhenIdle) + var trustedOrigins []string for _, origin := range strings.Split(os.Getenv("trustedOrigins"), ",") { if origin = strings.TrimSpace(origin); origin != "" { @@ -157,12 +179,14 @@ func LoadConfig() *Config { } return &Config{ - LogLevel: envLogLevel, - HttpListenAddress: addr, - CacheMaxAge: cacheMaxAgeInt, - ScanTimeout: scanTimeoutInt, - VehicleDataCacheTime: vehicleDataCacheTimeInt, - TrustedOrigins: trustedOrigins, + LogLevel: envLogLevel, + HttpListenAddress: addr, + CacheMaxAge: cacheMaxAgeInt, + ScanTimeout: scanTimeoutInt, + VehicleDataCacheTime: vehicleDataCacheTimeInt, + TrustedOrigins: trustedOrigins, + ConnectionTimeout: connectionTimeoutInt, + ReleaseAdapterWhenIdle: releaseAdapterWhenIdle, } } diff --git a/docs/environment_variables.md b/docs/environment_variables.md index a9694fc..5a2ec88 100644 --- a/docs/environment_variables.md +++ b/docs/environment_variables.md @@ -37,6 +37,18 @@ You need it in two cases, because the proxy otherwise answers `403 Forbidden`: Case, a trailing slash and the default ports `:80`/`:443` do not matter. Several origins: `trustedOrigins=http://nas.local:8080,https://home.example.com`. +## connectionTimeout + +*(unreleased)* + +The number of seconds a Bluetooth connection to the vehicle stays open after a command, so that further commands (for example from evcc) are sent without connecting again. A lower value frees the Bluetooth adapter sooner, which matters with `releaseAdapterWhenIdle`; a higher value means fewer reconnects. (Default: 29) + +## releaseAdapterWhenIdle + +*(unreleased)* + +If `true`, the proxy hands the Bluetooth adapter back to the system (BlueZ) whenever no connection to the vehicle is open, so other Bluetooth services on the same host can use it, for example a door lock. By default the proxy keeps the adapter for itself until it stops. With this option every new connection sets up the adapter again, which takes a little longer. Combine it with a lower `connectionTimeout` (for example `10`) to free the adapter sooner after a command. (Default: false) + # Example ## Docker compose diff --git a/go.mod b/go.mod index daaeb39..04eaf9e 100644 --- a/go.mod +++ b/go.mod @@ -39,4 +39,4 @@ require ( replace github.com/teslamotors/vehicle-command => github.com/wimaha/vehicle-command v0.0.8 -replace github.com/go-ble/ble => github.com/wimaha/ble_BleConnectFix v0.0.0-20240822192426-3f74826c1268 +replace github.com/go-ble/ble => github.com/wimaha/ble_BleConnectFix v0.0.0-20261018210806-b933489a83fc diff --git a/go.sum b/go.sum index 2b8e43d..ef65df7 100644 --- a/go.sum +++ b/go.sum @@ -72,8 +72,8 @@ github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/ github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= github.com/urfave/cli v1.22.2/go.mod h1:Gos4lmkARVdJ6EkW0WaNv/tZAAMe9V7XWyB60NtXRu0= -github.com/wimaha/ble_BleConnectFix v0.0.0-20240822192426-3f74826c1268 h1:36sDJ2qts2oT/Fy/Wi6MR15C+LHl2+ZUzLp6UaqhS9c= -github.com/wimaha/ble_BleConnectFix v0.0.0-20240822192426-3f74826c1268/go.mod h1:fFJl/jD/uyILGBeD5iQ8tYHrPlJafyqCJzAyTHNJ1Uk= +github.com/wimaha/ble_BleConnectFix v0.0.0-20261018210806-b933489a83fc h1:jevReITmxQ1l/Q8S7s9yP/YTiae2gyP5B3Pyg59x9QQ= +github.com/wimaha/ble_BleConnectFix v0.0.0-20261018210806-b933489a83fc/go.mod h1:fFJl/jD/uyILGBeD5iQ8tYHrPlJafyqCJzAyTHNJ1Uk= github.com/wimaha/vehicle-command v0.0.8 h1:hFjih8U2OTI9sKrPEx3A0cKSCq/bN5pVVMeM9RSvhWw= github.com/wimaha/vehicle-command v0.0.8/go.mod h1:liN6VG6MCc7m02wFaBm2sQT6MYGm/dJua6bG00QSpnA= github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e h1:JVG44RsyaB9T2KIHavMF/ppJZNG9ZpyihvCd0w101no= diff --git a/internal/api/handlers/html.go b/internal/api/handlers/html.go index f51257a..47e8616 100644 --- a/internal/api/handlers/html.go +++ b/internal/api/handlers/html.go @@ -196,8 +196,7 @@ func RemoveKeys(w http.ResponseWriter, r *http.Request) { } } - control.CloseBleControl() - control.SetupBleControl() // Reinitialize with new active key + control.SetupBleControl() // Reinitialize with new active key; stops the previous loop http.Redirect(w, r, "/dashboard", http.StatusSeeOther) } @@ -319,8 +318,7 @@ func ActivateKey(w http.ResponseWriter, r *http.Request) { Message: fmt.Sprintf("Active key changed to '%s'.", control.GetKeyRoleDisplayName(role)), Type: models.Success, }) - // Reinitialize BLE control with new active key - control.CloseBleControl() + // Reinitialize BLE control with new active key; stops the previous loop control.SetupBleControl() } } diff --git a/internal/api/handlers/tesla.go b/internal/api/handlers/tesla.go index b1a3a6c..11224d7 100644 --- a/internal/api/handlers/tesla.go +++ b/internal/api/handlers/tesla.go @@ -47,13 +47,15 @@ func commonDefer(w http.ResponseWriter, response *models.Response) { logging.Debug("Response", "Command", response.Command, "Status", status, "Result", response.Result, "Reason", response.Reason) } -func checkBleControl(response *models.Response) bool { - if control.BleControlInstance == nil { +// bleControl returns the active BleControl, read once so a key change during the request +// cannot swap it out halfway. If there is none, it sets the response reason and returns nil. +func bleControl(response *models.Response) *control.BleControl { + bc := control.Instance() + if bc == nil { response.Reason = "BleControl is not initialized. Maybe private.pem is missing." response.Result = false - return false } - return true + return bc } func Command(w http.ResponseWriter, r *http.Request) { @@ -72,7 +74,8 @@ func Command(w http.ResponseWriter, r *http.Request) { defer commonDefer(w, &response) - if !checkBleControl(&response) { + bc := bleControl(&response) + if bc == nil { return } @@ -106,7 +109,7 @@ func Command(w http.ResponseWriter, r *http.Request) { apiResponse.Ctx = r.Context() wg.Add(1) - control.BleControlInstance.PushCommand(command, vin, body, &apiResponse, autoWakeup) + bc.PushCommand(command, vin, body, &apiResponse, autoWakeup) wg.Wait() @@ -121,7 +124,12 @@ func Command(w http.ResponseWriter, r *http.Request) { return } - control.BleControlInstance.PushCommand(command, vin, body, nil, autoWakeup) + // Nobody waits for the outcome, but a command that was not even queued must not look accepted. + if err := bc.PushCommand(command, vin, body, nil, autoWakeup); err != nil { + response.Result = false + response.Reason = err.Error() + return + } response.Result = true response.Reason = "The command was successfully received and will be processed shortly." } @@ -161,7 +169,8 @@ func VehicleData(w http.ResponseWriter, r *http.Request) { defer commonDefer(w, &response) - if !checkBleControl(&response) { + bc := bleControl(&response) + if bc == nil { return } @@ -222,7 +231,7 @@ func VehicleData(w http.ResponseWriter, r *http.Request) { wg.Add(1) autoWakeup := r.URL.Query().Get("wakeup") == "true" - control.BleControlInstance.PushCommand(command, vin, map[string]interface{}{"endpoints": endpoints}, &apiResponse, autoWakeup) + bc.PushCommand(command, vin, map[string]interface{}{"endpoints": endpoints}, &apiResponse, autoWakeup) wg.Wait() @@ -302,46 +311,50 @@ func BodyControllerState(w http.ResponseWriter, r *http.Request) { defer commonDefer(w, &response) - if !checkBleControl(&response) { + bc := bleControl(&response) + if bc == nil { return } - var apiResponse models.ApiResponse - + // Through the queue like every other request, so it never opens a second BLE connection. If + // a connection to the car is already open, it is answered on it right away. It only needs + // VCSEC, so it does not wake the car. ctx, cancel := context.WithTimeout(r.Context(), 15*time.Second) - apiResponse.Ctx = ctx defer cancel() - cmd := &commands.Command{ + var apiResponse models.ApiResponse + wg := sync.WaitGroup{} + apiResponse.Wait = &wg + apiResponse.Ctx = ctx + wg.Add(1) + bc.Push(commands.Command{ Command: "body-controller-state", Domain: commands.Domain.VCSEC, Vin: vin, Response: &apiResponse, - } - car, _, err := control.BleControlInstance.TryConnectToVehicle(ctx, cmd) - if err == nil { - //Successful - defer car.Disconnect() + }) - _, err, _ := control.BleControlInstance.ExecuteCommand(car, cmd, context.Background()) - if err != nil { - response.Result = false - response.Reason = err.Error() - return - } + // The loop answers once it reaches the command; do not wait past the timeout for that. + answered := make(chan struct{}) + go func() { + wg.Wait() + close(answered) + }() + select { + case <-answered: + case <-ctx.Done(): + response.Result = false + response.Reason = fmt.Sprintf("failed to get body controller state: %s", ctx.Err()) + return + } + if apiResponse.Result { SetCacheControl(w, config.AppConfig.CacheMaxAge) - - if apiResponse.Result { - response.Result = true - response.Reason = "The request was successfully processed." - response.Response = apiResponse.Response - } else { - response.Result = false - response.Reason = apiResponse.Error - } + response.Result = true + response.Reason = "The request was successfully processed." + response.Response = apiResponse.Response } else { response.Result = false - response.Reason = err.Error() + response.Reason = apiResponse.Error } } diff --git a/internal/api/handlers/tesla_test.go b/internal/api/handlers/tesla_test.go index f05c934..44f141d 100644 --- a/internal/api/handlers/tesla_test.go +++ b/internal/api/handlers/tesla_test.go @@ -23,7 +23,7 @@ import ( ) // End-to-end: HTTP request -> router -> handler -> command queue -> simulated car -> HTTP answer. -// The handlers use the global control.BleControlInstance, so these tests do not run in parallel. +// The handlers use the global control.Instance(), so these tests do not run in parallel. // Each test uses a fresh VIN because the vehicle_data cache is global as well. var vinCounter atomic.Int32 @@ -50,12 +50,12 @@ func startProxy(t *testing.T, vin string) *proxy { bc.Timing = control.Timing{LoopDelay: time.Millisecond, RetryDelay: time.Millisecond, ConnectionTimeout: 200 * time.Millisecond} go bc.Loop() - previous := control.BleControlInstance - control.BleControlInstance = bc + previous := control.Replace(bc) server := httptest.NewServer(routes.SetupRoutes(fstest.MapFS{}, os.DirFS("../../.."), nil)) t.Cleanup(func() { server.Close() - control.BleControlInstance = previous + control.Replace(previous) + bc.Stop() }) return &proxy{t: t, car: car, server: server} } @@ -126,6 +126,23 @@ func TestHTTPCommandWithoutWait(t *testing.T) { } } +// A command without wait=true that cannot even be queued must not be reported as accepted. +func TestHTTPCommandWithoutWaitReportsRejection(t *testing.T) { + vin := newVin() + p := startProxy(t, vin) + control.Instance().Stop() // as during a key change + + code, resp := p.do("POST", "/api/1/vehicles/"+vin+"/command/flash_lights", "") + + wantStatus(t, code, resp, http.StatusServiceUnavailable) + if resp.Result || !strings.Contains(resp.Reason, "restarted") { + t.Errorf("response = %+v, want the rejection", resp) + } + if n := len(p.car.Calls()); n != 0 { + t.Errorf("car got %v, want no contact", p.car.Methods()) + } +} + func TestHTTPRejectsWithoutContactingCar(t *testing.T) { vin := newVin() p := startProxy(t, vin) @@ -134,6 +151,10 @@ func TestHTTPRejectsWithoutContactingCar(t *testing.T) { {"/command/self_destruct?wait=true", "", "not supported"}, {"/command/set_temps?wait=true", `{"driver_temp": 40}`, "out of range"}, {"/command/actuate_trunk?wait=true", `{"which_trunk": "side"}`, "which_trunk"}, + // used to come back as result=true without a body + {"/command/set_charging_amps?wait=true", "", "charging_amps missing"}, + {"/command/set_charge_limit?wait=true", `{"percent": "eighty"}`, "percent parsing error"}, + {"/command/set_sentry_mode?wait=true", `{}`, "on missing"}, } { code, resp := p.do("POST", "/api/1/vehicles/"+vin+tc.path, tc.body) wantStatus(t, code, resp, http.StatusServiceUnavailable) @@ -242,8 +263,14 @@ func TestHTTPBodyControllerState(t *testing.T) { if n := p.car.Count("Wakeup"); n != 0 { t.Errorf("Wakeup count = %d, want 0: reading the body controller must not wake the car", n) } - if n := p.car.Count("Disconnect"); n != 1 { - t.Errorf("Disconnect count = %d, want 1", n) + // It runs through the command queue, so the connection stays open for further commands and + // closes after the connection timeout like any other. + deadline := time.Now().Add(3 * time.Second) + for p.car.Count("Disconnect") != 1 { + if time.Now().After(deadline) { + t.Fatalf("Disconnect count = %d, want 1 after the connection timeout", p.car.Count("Disconnect")) + } + time.Sleep(5 * time.Millisecond) } } diff --git a/internal/ble/control/control.go b/internal/ble/control/control.go index 5a6459b..e1f9a85 100644 --- a/internal/ble/control/control.go +++ b/internal/ble/control/control.go @@ -7,6 +7,7 @@ import ( "os" "strings" "sync" + "sync/atomic" "time" "github.com/teslamotors/vehicle-command/pkg/connector/ble" @@ -19,22 +20,64 @@ import ( "github.com/wimaha/TeslaBleHttpProxy/internal/tesla/commands" ) -var BleControlInstance *BleControl = nil +// instance is the active BleControl. Handlers read it once per request with Instance(); the +// dashboard swaps it when the active key changes. +var ( + instance atomic.Pointer[BleControl] + setupMu sync.Mutex +) + +// bleMu is held for as long as a BLE connection is open, and while the adapter is released. +// The command loop, a loop being replaced after a key change, and SendKeysToVehicle each hold +// it, so there is never more than one connection and the adapter is never closed under one. +var bleMu sync.Mutex +// Instance returns the active BleControl, or nil if no key is loaded. +func Instance() *BleControl { + return instance.Load() +} + +// Replace makes bc the active BleControl and returns the previous one. It neither starts nor +// stops a loop; SetupBleControl and CloseBleControl do that. +func Replace(bc *BleControl) *BleControl { + return instance.Swap(bc) +} + +// SetupBleControl loads the active key, starts a new command loop and stops the previous one. func SetupBleControl() { - var err error - if BleControlInstance, err = NewBleControl(); err != nil { + setupMu.Lock() + defer setupMu.Unlock() + + bc, err := NewBleControl() + if err != nil { logging.Warn("BleControl could not be initialized!") + bc = nil } else { - go BleControlInstance.Loop() + go bc.Loop() logging.Info("BleControl initialized") } + if previous := Replace(bc); previous != nil { + previous.Stop() + } } +// CloseBleControl stops the active command loop. func CloseBleControl() { - BleControlInstance = nil + setupMu.Lock() + defer setupMu.Unlock() + + if previous := Replace(nil); previous != nil { + previous.Stop() + } } +// errStopped answers commands that reach a BleControl after it was replaced. +var errStopped = errors.New("BLE control was restarted (active key changed), please retry") + +// errQueueFull answers commands while the queue is full, which means the car has been +// unreachable for a while and requests piled up. +var errQueueFull = errors.New("command queue is full, the vehicle is not reachable at the moment") + // Car is a connected vehicle: everything Send needs plus session handling. // *vehicle.Vehicle implements it; tests use internal/ble/fakecar. type Car interface { @@ -65,9 +108,22 @@ type BleControl struct { dial Dialer Timing Timing + // ReleaseAdapterWhenIdle hands the BLE adapter back to the system (BlueZ) whenever no + // connection is open, so other Bluetooth services on the host can use it. + ReleaseAdapterWhenIdle bool + releaseAdapter func() error + commandStack chan commands.Command providerStack chan commands.Command + // stopMu orders pushes against Stop: after Stop no command can enter the queue, so the + // loop can answer everything still queued and exit. + stopMu sync.RWMutex + stopped bool + done chan struct{} + + closeRequest chan struct{} // see RequestClose + // Cache to track when each vehicle was last confirmed awake lastAwakeTime map[string]time.Time awakeTimeMu sync.RWMutex @@ -87,19 +143,29 @@ func NewBleControl() (*BleControl, error) { } logging.Debug("PrivateKeyFile loaded", "PrivateKeyFile", privateKeyFile, "Role", GetActiveKeyRole()) - return New(privateKey, nil), nil + bc := New(privateKey, nil) + if config.AppConfig != nil { + if config.AppConfig.ConnectionTimeout > 0 { + bc.Timing.ConnectionTimeout = time.Duration(config.AppConfig.ConnectionTimeout) * time.Second + } + bc.ReleaseAdapterWhenIdle = config.AppConfig.ReleaseAdapterWhenIdle + } + return bc, nil } // New returns a BleControl that connects through dial, or over BLE if dial is nil. // Without a private key only unauthenticated requests (add-key-request) are possible. func New(privateKey protocol.ECDHPrivateKey, dial Dialer) *BleControl { bc := &BleControl{ - privateKey: privateKey, - dial: dial, - Timing: DefaultTiming, - commandStack: make(chan commands.Command, 50), - providerStack: make(chan commands.Command), - lastAwakeTime: make(map[string]time.Time), + privateKey: privateKey, + dial: dial, + Timing: DefaultTiming, + releaseAdapter: ble.CloseAdapter, + commandStack: make(chan commands.Command, 50), + providerStack: make(chan commands.Command), + done: make(chan struct{}), + closeRequest: make(chan struct{}, 1), + lastAwakeTime: make(map[string]time.Time), } if bc.dial == nil { bc.dial = bc.bleDial @@ -109,15 +175,34 @@ func New(privateKey protocol.ECDHPrivateKey, dial Dialer) *BleControl { func (bc *BleControl) Loop() { var retryCommand *commands.Command + defer func() { + if retryCommand != nil { + failCommand(retryCommand, errStopped) + } + bc.failQueued() + logging.Debug("Command loop stopped") + }() for { time.Sleep(bc.Timing.LoopDelay) + if bc.isStopped() { + return + } if retryCommand != nil { logging.Info("Retrying command", "Command", retryCommand.Command, "Body", retryCommand.Body) retryCommand = bc.connectToVehicleAndOperateConnection(retryCommand) } else { + if bc.ReleaseAdapterWhenIdle { + bleMu.Lock() + if err := bc.releaseAdapter(); err != nil { + logging.Debug("Failed to release BLE adapter", "error", err) + } + bleMu.Unlock() + } logging.Debug("Waiting for next command ...") // Wait for the next command select { + case <-bc.done: + return case command, ok := <-bc.providerStack: if ok { retryCommand = bc.connectToVehicleAndOperateConnection(&command) @@ -131,13 +216,90 @@ func (bc *BleControl) Loop() { } } -func (bc *BleControl) PushCommand(command string, vin string, body map[string]interface{}, response *models.ApiResponse, autoWakeup bool) { - bc.commandStack <- commands.Command{ +// Stop ends the command loop. An open connection is closed, and commands that are queued or +// pushed later are answered with an error instead of waiting forever. +func (bc *BleControl) Stop() { + bc.stopMu.Lock() + defer bc.stopMu.Unlock() + if !bc.stopped { + bc.stopped = true + close(bc.done) + } +} + +func (bc *BleControl) isStopped() bool { + select { + case <-bc.done: + return true + default: + return false + } +} + +// failQueued answers every queued command with errStopped. Called by the loop after Stop, when +// no further command can be pushed. +func (bc *BleControl) failQueued() { + for { + select { + case command := <-bc.commandStack: + failCommand(&command, errStopped) + default: + return + } + } +} + +// failCommand reports err to whoever waits for the command. +func failCommand(command *commands.Command, err error) { + if command.Response == nil { + // Sent without wait=true: nobody is waiting, so at least say that it was not executed. + logging.Warn("Command not executed", "Command", command.Command, "Vin", command.Vin, "Error", err) + return + } + command.Response.Error = err.Error() + command.Response.Result = false + if command.Response.Wait != nil { + command.Response.Wait.Done() + } +} + +// PushCommand queues a command; see Push for the returned error. +func (bc *BleControl) PushCommand(command string, vin string, body map[string]interface{}, response *models.ApiResponse, autoWakeup bool) error { + return bc.Push(commands.Command{ Command: command, Vin: vin, Body: body, Response: response, AutoWakeup: autoWakeup, + }) +} + +// Push queues a command. After Stop, or if the queue is full, the command is rejected: the +// error is returned and also reported to command.Response, if any. Push never blocks, so Stop +// cannot be held up by a waiting push. +func (bc *BleControl) Push(command commands.Command) error { + bc.stopMu.RLock() + defer bc.stopMu.RUnlock() + err := errStopped + if !bc.stopped { + select { + case bc.commandStack <- command: + return nil + default: + err = errQueueFull + } + } + failCommand(&command, err) + return err +} + +// RequestClose asks the loop to close its connection once the current command is done, instead +// of keeping it open for the connection timeout. Used before opening a connection outside the +// loop (SendKeysToVehicle). +func (bc *BleControl) RequestClose() { + select { + case bc.closeRequest <- struct{}{}: + default: // already requested } } @@ -164,6 +326,8 @@ func (bc *BleControl) markVehicleAwake(vin string) { } func (bc *BleControl) connectToVehicleAndOperateConnection(firstCommand *commands.Command) *commands.Command { + bleMu.Lock() + defer bleMu.Unlock() logging.Info("Connecting to Vehicle ...") //defer log.Debug("connecting to Vehicle done") @@ -172,14 +336,11 @@ func (bc *BleControl) connectToVehicleAndOperateConnection(firstCommand *command var lastErr error commandError := func(err error) *commands.Command { - logging.Error("Cannot connect to vehicle", "Error", err) - if firstCommand.Response != nil { - firstCommand.Response.Error = err.Error() - firstCommand.Response.Result = false - if firstCommand.Response.Wait != nil { - firstCommand.Response.Wait.Done() - } + if bc.isStopped() { + err = errStopped } + logging.Error("Cannot connect to vehicle", "Error", err) + failCommand(firstCommand, err) return nil } @@ -196,6 +357,18 @@ func (bc *BleControl) connectToVehicleAndOperateConnection(firstCommand *command parentCtx = context.Background() } + // Stop also ends a connection attempt (scan, retries), so after a key change the old loop + // frees Bluetooth at once even while the car is out of range. + parentCtx, cancelParent := context.WithCancel(parentCtx) + defer cancelParent() + go func() { + select { + case <-bc.done: + cancelParent() + case <-parentCtx.Done(): + } + }() + for i := 0; i < retryCount; i++ { if i > 0 { logging.Warn("Retry error", "error", lastErr) @@ -474,6 +647,9 @@ func (bc *BleControl) operateConnection(car Car, firstCommand *commands.Command) return cmd } + // TryConnectToVehicle starts Infotainment only for commands that need it. + infotainment := needsInfotainment(firstCommand) + // If wake_up command executed successfully, upgrade session to include Infotainment // for subsequent commands that might need it if firstCommand.Command == "wake_up" { @@ -487,6 +663,7 @@ func (bc *BleControl) operateConnection(car Car, firstCommand *commands.Command) logging.Debug("Failed to upgrade session to Infotainment, subsequent commands may fail", "Error", err) } else { logging.Debug("Session upgraded to include Infotainment") + infotainment = true } } @@ -496,6 +673,12 @@ func (bc *BleControl) operateConnection(car Car, firstCommand *commands.Command) logging.Debug("New VIN, closing connection ...") return true, command } + // A VCSEC-only connection (body-controller-state) cannot serve this command. Reconnect, + // which runs the wake-up and session logic for it. + if needsInfotainment(command) && !infotainment { + logging.Debug("Command needs Infotainment, reconnecting ...", "Command", command.Command) + return true, command + } cmd, err, ctx := bc.ExecuteCommand(car, command, connectionCtx) @@ -523,6 +706,12 @@ func (bc *BleControl) operateConnection(car Car, firstCommand *commands.Command) case <-connectionCtx.Done(): logging.Debug("Connection timeout ...") return nil + case <-bc.done: + logging.Debug("Command loop stopped, closing connection ...") + return nil + case <-bc.closeRequest: + logging.Debug("Connection needed elsewhere, closing connection ...") + return nil case command, ok := <-bc.providerStack: if !ok { return nil @@ -545,6 +734,12 @@ func (bc *BleControl) operateConnection(car Car, firstCommand *commands.Command) } } +// needsInfotainment reports whether a command needs an Infotainment session. wake_up and +// VCSEC commands (body-controller-state) work with VCSEC alone. +func needsInfotainment(command *commands.Command) bool { + return command.Domain != commands.Domain.VCSEC && command.Command != "wake_up" +} + func (bc *BleControl) ExecuteCommand(car commands.Vehicle, command *commands.Command, connectionCtx context.Context) (retryCommand *commands.Command, retErr error, ctx context.Context) { logging.Info("Executing command", "Command", command.Command, "Body", command.Body) if command.Response != nil && command.Response.Ctx != nil { @@ -621,14 +816,11 @@ func (bc *BleControl) ExecuteCommand(car commands.Vehicle, command *commands.Com if errors.As(err, &badInput) { return nil, err, ctx } - // A send that must not be repeated (rear trunk toggle) failed: tell the caller instead - // of reporting success. The returned command is nil, so it is never re-run. - var notRetried *commands.NotRetriedError - if errors.As(err, ¬Retried) { - logging.Error("Failed, not retried", "Command", command.Command, "Body", command.Body, "Error", err) - return nil, err, ctx - } - return nil, nil, ctx + // Any other error that is not worth retrying (a toggle that must not be repeated, an + // unknown command, a missing key file) is reported, never turned into success. The + // returned command is nil, so it is not re-run. + logging.Error("Failed, not retried", "Command", command.Command, "Body", command.Body, "Error", err) + return nil, err, ctx } if strings.Contains(err.Error(), "closed pipe") { diff --git a/internal/ble/control/control_lifecycle_test.go b/internal/ble/control/control_lifecycle_test.go new file mode 100644 index 0000000..75a5a71 --- /dev/null +++ b/internal/ble/control/control_lifecycle_test.go @@ -0,0 +1,394 @@ +package control + +import ( + "context" + "errors" + "reflect" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/wimaha/TeslaBleHttpProxy/internal/api/models" + "github.com/wimaha/TeslaBleHttpProxy/internal/ble/fakecar" + "github.com/wimaha/TeslaBleHttpProxy/internal/tesla/commands" +) + +// Stopping, replacing and sharing the BLE connection: what a key change, body-controller-state +// and releaseAdapterWhenIdle rely on. + +// newControl returns a BleControl for the cars without starting its loop. +func newControl(t *testing.T, connectionTimeout time.Duration, cars ...*fakecar.Car) *BleControl { + t.Helper() + byVin := map[string]*fakecar.Car{} + for _, c := range cars { + byVin[c.Vin] = c + } + bc := New(fakecar.PrivateKey(), func(ctx context.Context, vin string) (Car, bool, error) { + c, ok := byVin[vin] + if !ok { + return nil, true, errors.New("vehicle is not in range") + } + car, retry, err := c.Dial(ctx, vin) + if err != nil { + return nil, retry, err + } + return car, false, nil + }) + bc.Timing = Timing{LoopDelay: time.Millisecond, RetryDelay: time.Millisecond, ConnectionTimeout: connectionTimeout} + bc.releaseAdapter = func() error { return nil } + t.Cleanup(bc.Stop) + return bc +} + +// pushWait queues the command like a handler with wait=true and returns a channel that is +// closed once the command is answered. +func pushWait(bc *BleControl, command commands.Command) (*models.ApiResponse, <-chan struct{}) { + wg := &sync.WaitGroup{} + resp := &models.ApiResponse{Wait: wg, Ctx: context.Background()} + command.Response = resp + wg.Add(1) + bc.Push(command) + answered := make(chan struct{}) + go func() { wg.Wait(); close(answered) }() + return resp, answered +} + +func waitAnswered(t *testing.T, what string, answered <-chan struct{}) { + t.Helper() + select { + case <-answered: + case <-time.After(3 * time.Second): + t.Fatalf("%s: no answer within 3s", what) + } +} + +func bodyControllerState(vin string) commands.Command { + return commands.Command{Command: "body-controller-state", Domain: commands.Domain.VCSEC, Vin: vin} +} + +func TestStopAnswersQueuedAndLaterCommands(t *testing.T) { + car := fakecar.New(vin1) + bc := newControl(t, time.Second, car) + + queued, queuedAnswered := pushWait(bc, commands.Command{Command: "flash_lights", Vin: vin1}) + bc.Stop() + later, laterAnswered := pushWait(bc, commands.Command{Command: "flash_lights", Vin: vin1}) + waitAnswered(t, "push after Stop", laterAnswered) + mustFail(t, later, "restarted") + + // The loop of a stopped control answers what was queued before and exits. + go bc.Loop() + waitAnswered(t, "queued before Stop", queuedAnswered) + mustFail(t, queued, "restarted") + if n := car.Count("Dial"); n != 0 { + t.Errorf("Dial count = %d, want 0", n) + } +} + +func TestStopClosesOpenConnection(t *testing.T) { + car := fakecar.New(vin1) + bc := newControl(t, time.Minute, car) + go bc.Loop() + + resp, answered := pushWait(bc, commands.Command{Command: "flash_lights", Vin: vin1}) + waitAnswered(t, "flash_lights", answered) + mustSucceed(t, resp) + + bc.Stop() + eventually(t, "connection closed after Stop", func() bool { return car.Count("Disconnect") == 1 }) +} + +// countedCar tracks how many connections are open at the same time. +type countedCar struct { + *fakecar.Car + open, maxOpen *atomic.Int32 +} + +func (c countedCar) Disconnect() { + c.open.Add(-1) + c.Car.Disconnect() +} + +// countingDialer dials car and records the highest number of simultaneously open connections. +func countingDialer(car *fakecar.Car, open, maxOpen *atomic.Int32) Dialer { + return func(ctx context.Context, vin string) (Car, bool, error) { + c, retry, err := car.Dial(ctx, vin) + if err != nil { + return nil, retry, err + } + n := open.Add(1) + for m := maxOpen.Load(); n > m && !maxOpen.CompareAndSwap(m, n); m = maxOpen.Load() { + } + return countedCar{Car: c, open: open, maxOpen: maxOpen}, false, nil + } +} + +// After a key change the new loop takes over without a second connection being open at the +// same time: the old loop closes its connection when stopped, the new one waits for that. +func TestReplacedLoopHandsOverConnection(t *testing.T) { + car := fakecar.New(vin1) + var open, maxOpen atomic.Int32 + old := newControl(t, time.Minute, car) + old.dial = countingDialer(car, &open, &maxOpen) + go old.Loop() + resp, answered := pushWait(old, commands.Command{Command: "flash_lights", Vin: vin1}) + waitAnswered(t, "old loop", answered) + mustSucceed(t, resp) + + replacement := newControl(t, time.Minute, car) + replacement.dial = countingDialer(car, &open, &maxOpen) + go replacement.Loop() + resp, answered = pushWait(replacement, commands.Command{Command: "flash_lights", Vin: vin1}) + time.Sleep(20 * time.Millisecond) // the new loop is now waiting for the old connection + old.Stop() + + waitAnswered(t, "new loop", answered) + mustSucceed(t, resp) + if n := car.Count("Dial"); n != 2 { + t.Errorf("Dial count = %d, want 2 (one per loop)", n) + } + if n := maxOpen.Load(); n != 1 { + t.Errorf("%d connections were open at the same time, want 1", n) + } +} + +func TestInstanceSwapIsRaceFree(t *testing.T) { + previous := Replace(nil) + t.Cleanup(func() { Replace(previous) }) + + var wg sync.WaitGroup + for i := 0; i < 4; i++ { + wg.Add(2) + go func() { + defer wg.Done() + for j := 0; j < 200; j++ { + bc := New(nil, nil) + if old := Replace(bc); old != nil { + old.Stop() + } + } + }() + go func() { + defer wg.Done() + for j := 0; j < 200; j++ { + // Read once, like the handlers; a stopped instance answers right away. + if bc := Instance(); bc != nil { + bc.Stop() + bc.Push(commands.Command{Command: "flash_lights", Vin: vin1}) + } + } + }() + } + wg.Wait() +} + +// body-controller-state goes through the queue: it reuses an open connection instead of +// opening a second one. +func TestBodyControllerStateUsesOpenConnection(t *testing.T) { + car := fakecar.New(vin1) + bc := newControl(t, time.Minute, car) + go bc.Loop() + + resp, answered := pushWait(bc, commands.Command{Command: "flash_lights", Vin: vin1}) + waitAnswered(t, "flash_lights", answered) + mustSucceed(t, resp) + + resp, answered = pushWait(bc, bodyControllerState(vin1)) + waitAnswered(t, "body-controller-state", answered) + mustSucceed(t, resp) + if len(resp.Response) == 0 { + t.Error("body-controller-state returned no data") + } + if n := car.Count("Dial"); n != 1 { + t.Errorf("Dial count = %d, want 1: body-controller-state must use the open connection", n) + } +} + +// A VCSEC-only connection (opened for body-controller-state) must not serve a command that +// needs Infotainment; that command reconnects and wakes the car as usual. +func TestInfotainmentCommandReconnectsAfterBodyControllerState(t *testing.T) { + car := fakecar.New(vin1) + car.Asleep = true + bc := newControl(t, time.Minute, car) + go bc.Loop() + + resp, answered := pushWait(bc, bodyControllerState(vin1)) + waitAnswered(t, "body-controller-state", answered) + mustSucceed(t, resp) + if n := car.Count("Wakeup"); n != 0 { + t.Fatalf("Wakeup count = %d, want 0: body-controller-state must not wake the car", n) + } + + resp, answered = pushWait(bc, commands.Command{Command: "charge_start", Vin: vin1, AutoWakeup: true}) + waitAnswered(t, "charge_start", answered) + mustSucceed(t, resp) + + if n := car.Count("Dial"); n != 2 { + t.Errorf("Dial count = %d, want 2 (reconnect for Infotainment)", n) + } + if n := car.Count("Wakeup"); n != 1 { + t.Errorf("Wakeup count = %d, want 1", n) + } + if got := sessions(car); !reflect.DeepEqual(got[len(got)-1], withInfotainment) { + t.Errorf("last session = %v, want %v", got[len(got)-1], withInfotainment) + } +} + +func TestAdapterReleasedOnlyWhileIdle(t *testing.T) { + car := fakecar.New(vin1) + bc := newControl(t, 50*time.Millisecond, car) + bc.ReleaseAdapterWhenIdle = true + var releases, whileConnected atomic.Int32 + bc.releaseAdapter = func() error { + releases.Add(1) + if car.Count("Dial") != car.Count("Disconnect") { + whileConnected.Add(1) + } + return nil + } + go bc.Loop() + + eventually(t, "release before the first command", func() bool { return releases.Load() >= 1 }) + before := releases.Load() + resp, answered := pushWait(bc, commands.Command{Command: "flash_lights", Vin: vin1}) + waitAnswered(t, "flash_lights", answered) + mustSucceed(t, resp) + eventually(t, "release after the connection closed", func() bool { return releases.Load() > before }) + + if n := whileConnected.Load(); n != 0 { + t.Errorf("adapter released %d times while a connection was open", n) + } +} + +func TestAdapterKeptByDefault(t *testing.T) { + car := fakecar.New(vin1) + bc := newControl(t, 20*time.Millisecond, car) + var releases atomic.Int32 + bc.releaseAdapter = func() error { releases.Add(1); return nil } + go bc.Loop() + + resp, answered := pushWait(bc, commands.Command{Command: "flash_lights", Vin: vin1}) + waitAnswered(t, "flash_lights", answered) + mustSucceed(t, resp) + eventually(t, "connection closed", func() bool { return car.Count("Disconnect") == 1 }) + time.Sleep(20 * time.Millisecond) + + if n := releases.Load(); n != 0 { + t.Errorf("adapter released %d times without releaseAdapterWhenIdle", n) + } +} + +// An error that is not worth retrying must reach the caller instead of being reported as success. +func TestNonRetriedErrorIsReported(t *testing.T) { + wait := &sync.WaitGroup{} + wait.Add(1) + cmd := &commands.Command{ + Command: "no_such_command", + Response: &models.ApiResponse{Wait: wait, Ctx: context.Background()}, + } + + retry, err, _ := (&BleControl{}).ExecuteCommand(nil, cmd, context.Background()) + + if err == nil || cmd.Response.Result { + t.Fatalf("err = %v, result = %v, want the error reported", err, cmd.Response.Result) + } + if retry != nil { + t.Errorf("a non-retried error must not be re-run") + } + wait.Wait() +} + +// After a key change while the car is out of range, the old loop must not keep Bluetooth busy +// with its connection attempts (SendKeysToVehicle would wait for it). +func TestStopEndsConnectionAttempt(t *testing.T) { + bc := newControl(t, time.Second) // no car: every dial fails and is retried + bc.Timing.RetryDelay = time.Minute + go bc.Loop() + + resp, answered := pushWait(bc, commands.Command{Command: "flash_lights", Vin: vin1}) + time.Sleep(20 * time.Millisecond) // first attempt failed, now waiting to retry + bc.Stop() + + waitAnswered(t, "command during connection attempt", answered) + mustFail(t, resp, "restarted") + if !lockBLE(time.Second) { + t.Fatal("bleMu still held after Stop") + } + bleMu.Unlock() +} + +// A full queue answers at once instead of blocking the caller, so Stop is never held up. +func TestFullQueueRejectsAndStopDoesNotHang(t *testing.T) { + bc := newControl(t, time.Second, fakecar.New(vin1)) // loop not started: nothing drains the queue + for i := 0; i < cap(bc.commandStack); i++ { + bc.Push(commands.Command{Command: "flash_lights", Vin: vin1}) + } + resp, answered := pushWait(bc, commands.Command{Command: "flash_lights", Vin: vin1}) + waitAnswered(t, "push into a full queue", answered) + mustFail(t, resp, "queue is full") + // Without a response (no wait=true) the caller learns it from the returned error. + if err := bc.Push(commands.Command{Command: "flash_lights", Vin: vin1}); !errors.Is(err, errQueueFull) { + t.Errorf("Push into a full queue = %v, want errQueueFull", err) + } + + stopped := make(chan struct{}) + go func() { bc.Stop(); close(stopped) }() + select { + case <-stopped: + case <-time.After(time.Second): + t.Fatal("Stop blocked with a full queue") + } +} + +// A caller that gave up (body_controller_state after 15 s) does not reach the car later. +func TestCanceledRequestDoesNotReachCar(t *testing.T) { + car := fakecar.New(vin1) + bc := newControl(t, time.Second, car) + ctx, cancel := context.WithCancel(context.Background()) + cancel() + wg := &sync.WaitGroup{} + wg.Add(1) + resp := &models.ApiResponse{Wait: wg, Ctx: ctx} + command := bodyControllerState(vin1) + command.Response = resp + bc.Push(command) + go bc.Loop() + + answered := make(chan struct{}) + go func() { wg.Wait(); close(answered) }() + waitAnswered(t, "canceled body-controller-state", answered) + mustFail(t, resp, "canceled") + if n := len(car.Calls()); n != 0 { + t.Errorf("car got %v, want no contact", car.Methods()) + } +} + +// RequestClose ends an idle connection right away instead of after the connection timeout. +func TestRequestCloseEndsIdleConnection(t *testing.T) { + car := fakecar.New(vin1) + bc := newControl(t, time.Minute, car) + go bc.Loop() + resp, answered := pushWait(bc, commands.Command{Command: "flash_lights", Vin: vin1}) + waitAnswered(t, "flash_lights", answered) + mustSucceed(t, resp) + + bc.RequestClose() + eventually(t, "connection closed on request", func() bool { return car.Count("Disconnect") == 1 }) + if !lockBLE(time.Second) { + t.Fatal("bleMu still held after the connection closed") + } + bleMu.Unlock() +} + +func TestLockBLEGivesUp(t *testing.T) { + bleMu.Lock() + defer bleMu.Unlock() + start := time.Now() + if lockBLE(100 * time.Millisecond) { + t.Fatal("lockBLE succeeded while bleMu was held") + } + if d := time.Since(start); d > time.Second { + t.Errorf("lockBLE took %s, want about 100ms", d) + } +} diff --git a/internal/ble/control/control_sim_test.go b/internal/ble/control/control_sim_test.go index 3dea6a0..6f99a44 100644 --- a/internal/ble/control/control_sim_test.go +++ b/internal/ble/control/control_sim_test.go @@ -49,6 +49,7 @@ func startControl(t *testing.T, connectionTimeout time.Duration, cars ...*fakeca }) bc.Timing = Timing{LoopDelay: time.Millisecond, RetryDelay: time.Millisecond, ConnectionTimeout: connectionTimeout} go bc.Loop() + t.Cleanup(bc.Stop) return bc } diff --git a/internal/ble/control/key.go b/internal/ble/control/key.go index 5443d8b..988bae5 100644 --- a/internal/ble/control/key.go +++ b/internal/ble/control/key.go @@ -120,9 +120,49 @@ func RemoveKeyFiles() (error, error) { return err1, err2 } +// sendKeyWait is how long SendKeysToVehicle waits for the command loop to free the connection. +var sendKeyWait = 30 * time.Second + +// lockBLE takes bleMu, giving up after timeout. +func lockBLE(timeout time.Duration) bool { + deadline := time.Now().Add(timeout) + for !bleMu.TryLock() { + if time.Now().After(deadline) { + return false + } + time.Sleep(50 * time.Millisecond) + } + return true +} + func SendKeysToVehicle(vin string, role string) error { tempBleControl := New(nil, nil) + // Only one connection at a time: ask the command loop to close its connection and wait for + // it, but not forever. The 15 s for the request start after that. + if bc := Instance(); bc != nil { + bc.RequestClose() + } + if !lockBLE(sendKeyWait) { + return fmt.Errorf("Bluetooth is busy with another connection to the vehicle, please try again in a moment") + } + defer bleMu.Unlock() + if bc := Instance(); bc != nil { + // Drop a close request the loop did not need, so it does not end its next connection early. + select { + case <-bc.closeRequest: + default: + } + } + if config.AppConfig != nil && config.AppConfig.ReleaseAdapterWhenIdle { + // Deferred calls run last-in first-out: this runs after car.Disconnect, before Unlock. + defer func() { + if err := tempBleControl.releaseAdapter(); err != nil { + logging.Debug("Failed to release BLE adapter", "error", err) + } + }() + } + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) defer cancel() cmd := &commands.Command{ diff --git a/internal/tesla/commands/commands.go b/internal/tesla/commands/commands.go index f4669d4..63a3b76 100644 --- a/internal/tesla/commands/commands.go +++ b/internal/tesla/commands/commands.go @@ -82,18 +82,9 @@ func (command *Command) Send(ctx context.Context, car Vehicle) (shouldRetry bool } } case "set_sentry_mode": - var on bool - switch v := command.Body["on"].(type) { - case bool: - on = v - case string: - if onBool, err := strconv.ParseBool(v); err == nil { - on = onBool - } else { - return false, fmt.Errorf("on parsing error: %s", err) - } - default: - return false, fmt.Errorf("on missing in body") + on, err := parseBoolField(command.Body, "on") + if err != nil { + return false, err } if err := car.SetSentryMode(ctx, on); err != nil { return true, fmt.Errorf("failed to set sentry mode %s", err) @@ -121,35 +112,17 @@ func (command *Command) Send(ctx context.Context, car Vehicle) (shouldRetry bool return true, fmt.Errorf("failed to stop charge: %s", err) } case "set_charging_amps": - var chargingAmps int32 - switch v := command.Body["charging_amps"].(type) { - case float64: - chargingAmps = int32(v) - case string: - if chargingAmps64, err := strconv.ParseInt(v, 10, 32); err == nil { - chargingAmps = int32(chargingAmps64) - } else { - return false, fmt.Errorf("charing Amps parsing error: %s", err) - } - default: - return false, fmt.Errorf("charing Amps missing in body") + chargingAmps, err := parseInt32Field(command.Body, "charging_amps") + if err != nil { + return false, err } if err := car.SetChargingAmps(ctx, chargingAmps); err != nil { return true, fmt.Errorf("failed to set charging Amps to %d: %s", chargingAmps, err) } case "set_charge_limit": - var chargeLimit int32 - switch v := command.Body["percent"].(type) { - case float64: - chargeLimit = int32(v) - case string: - if chargeLimit64, err := strconv.ParseInt(v, 10, 32); err == nil { - chargeLimit = int32(chargeLimit64) - } else { - return false, fmt.Errorf("charing Amps parsing error: %s", err) - } - default: - return false, fmt.Errorf("charing Amps missing in body") + chargeLimit, err := parseInt32Field(command.Body, "percent") + if err != nil { + return false, err } if err := car.ChangeChargeLimit(ctx, chargeLimit); err != nil { return true, fmt.Errorf("failed to set charge limit to %d %%: %s", chargeLimit, err) @@ -318,7 +291,7 @@ func (command *Command) Send(ctx context.Context, car Vehicle) (shouldRetry bool // NotRetriedError marks a send that failed and is deliberately not retried, because repeating it // is not safe (the rear trunk is a toggle: the car may have acted before the reply was lost). -// Unlike other non-retried errors it is reported to the API caller, so a failed toggle does not +// Like every non-retried error it is reported to the API caller, so a failed toggle does not // come back as result=true. The connection is still closed as after any failed send. type NotRetriedError struct{ msg string } @@ -339,10 +312,55 @@ func ValidateBody(command string, body map[string]interface{}) error { case "actuate_trunk": _, err := parseWhichTrunk(body) return err + case "set_charging_amps": + _, err := parseInt32Field(body, "charging_amps") + return err + case "set_charge_limit": + _, err := parseInt32Field(body, "percent") + return err + case "set_sentry_mode": + _, err := parseBoolField(body, "on") + return err } return nil } +// parseInt32Field reads a required integer body field that may arrive as a JSON number or a string. +func parseInt32Field(body map[string]interface{}, key string) (int32, error) { + switch v := body[key].(type) { + case float64: + return int32(v), nil + case string: + n, err := strconv.ParseInt(strings.TrimSpace(v), 10, 32) + if err != nil { + return 0, invalidInput("%s parsing error: %s", key, err) + } + return int32(n), nil + case nil: + return 0, invalidInput("%s missing in body", key) + default: + return 0, invalidInput("%s has unsupported type %T", key, v) + } +} + +// parseBoolField reads a required boolean body field that may arrive as a JSON bool or a string. +func parseBoolField(body map[string]interface{}, key string) (bool, error) { + switch v := body[key].(type) { + case bool: + return v, nil + case string: + b, err := strconv.ParseBool(strings.TrimSpace(v)) + if err != nil { + return false, invalidInput("%s parsing error: %s", key, err) + } + return b, nil + case nil: + return false, invalidInput("%s missing in body", key) + default: + return false, invalidInput("%s has unsupported type %T", key, v) + } +} + // parseFloatField reads a numeric body field that may arrive as a JSON number or a string. func parseFloatField(body map[string]interface{}, key string) (float32, bool, error) { raw, ok := body[key] diff --git a/internal/tesla/commands/commands_parse_test.go b/internal/tesla/commands/commands_parse_test.go index 13aaa9e..6363b70 100644 --- a/internal/tesla/commands/commands_parse_test.go +++ b/internal/tesla/commands/commands_parse_test.go @@ -80,9 +80,19 @@ func TestValidateBody(t *testing.T) { {"actuate_trunk", map[string]interface{}{"which_trunk": "rear"}, false}, {"actuate_trunk", map[string]interface{}{"which_trunk": "side"}, true}, {"actuate_trunk", nil, true}, + {"set_charging_amps", map[string]interface{}{"charging_amps": 16.0}, false}, + {"set_charging_amps", map[string]interface{}{"charging_amps": " 16 "}, false}, + {"set_charging_amps", map[string]interface{}{"charging_amps": "sixteen"}, true}, + {"set_charging_amps", map[string]interface{}{"charging_amps": true}, true}, + {"set_charging_amps", nil, true}, + {"set_charge_limit", map[string]interface{}{"percent": "80"}, false}, + {"set_charge_limit", map[string]interface{}{}, true}, + {"set_sentry_mode", map[string]interface{}{"on": true}, false}, + {"set_sentry_mode", map[string]interface{}{"on": "false"}, false}, + {"set_sentry_mode", map[string]interface{}{"on": "maybe"}, true}, + {"set_sentry_mode", nil, true}, // commands without a pre-queue check pass through unchanged {"door_unlock", nil, false}, - {"set_charging_amps", nil, false}, } for _, tt := range tests { err := ValidateBody(tt.command, tt.body)