Refactor BLE connecting to allow scanning for vehicle presence

go.mod

@@ -5,7 +5,7 @@ go 1.23
 require (
 	github.com/99designs/keyring v1.2.2
 	github.com/cronokirby/saferith v0.33.0
-	github.com/go-ble/ble v0.0.0-20220207185428-60d1eecf2633
+	github.com/go-ble/ble v0.0.0-20240122180141-8c5522f54333
 	github.com/golang-jwt/jwt/v5 v5.2.1
 	github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510
 	golang.org/x/term v0.5.0

go.sum

@@ -13,8 +13,8 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/dvsekhvalnov/jose2go v1.6.0 h1:Y9gnSnP4qEI0+/uQkHvFXeD2PLPJeXEL+ySMEA2EjTY=
 github.com/dvsekhvalnov/jose2go v1.6.0/go.mod h1:QsHjhyTlD/lAVqn/NSbVZmSCGeDehTB/mPZadG+mhXU=
-github.com/go-ble/ble v0.0.0-20220207185428-60d1eecf2633 h1:ZrzoZQz1CF33SPHLkjRpnVuZwr9cO1lTEc4Js7SgBos=
-github.com/go-ble/ble v0.0.0-20220207185428-60d1eecf2633/go.mod h1:fFJl/jD/uyILGBeD5iQ8tYHrPlJafyqCJzAyTHNJ1Uk=
+github.com/go-ble/ble v0.0.0-20240122180141-8c5522f54333 h1:bQK6D51cNzMSTyAf0HtM30V2IbljHTDam7jru9JNlJA=
+github.com/go-ble/ble v0.0.0-20240122180141-8c5522f54333/go.mod h1:fFJl/jD/uyILGBeD5iQ8tYHrPlJafyqCJzAyTHNJ1Uk=
 github.com/godbus/dbus v0.0.0-20190726142602-4481cbc300e2 h1:ZpnhV/YsD2/4cESfV5+Hoeu/iUR3ruzNvZ+yQfO03a0=
 github.com/godbus/dbus v0.0.0-20190726142602-4481cbc300e2/go.mod h1:bBOAhwG1umN6/6ZUMtDFBMQR8jRg9O75tm9K00oMsK4=
 github.com/golang-jwt/jwt/v5 v5.2.1 h1:OuVbFODueb089Lh128TAcimifWaLhJwVflnrgM17wHk=

pkg/connector/ble/ble.go

@@ -5,6 +5,7 @@ package ble
 import (
 	"context"
 	"crypto/sha1"
+	"errors"
 	"fmt"
 	"strings"
 	"sync"
@@ -125,14 +126,97 @@ func (c *Connection) VIN() string {
 	return c.vin
 }
 
+func VehicleLocalName(vin string) string {
+	vinBytes := []byte(vin)
+	digest := sha1.Sum(vinBytes)
+	return fmt.Sprintf("S%02xC", digest[:8])
+}
+
+func initDevice() error {
+	var err error
+	// We don't want concurrent calls to NewConnection that would defeat
+	// the point of reusing the existing BLE device. Note that this is not
+	// an issue on MacOS, but multiple calls to newDevice() on Linux leads to failures.
+	if device != nil {
+		log.Debug("Reusing existing BLE device")
+	} else {
+		log.Debug("Creating new BLE device")
+		device, err = newDevice()
+		if err != nil {
+			return fmt.Errorf("failed to find a BLE device: %s", err)
+		}
+		ble.SetDefaultDevice(device)
+	}
+	return nil
+}
+
+type Advertisement = ble.Advertisement
+
+func ScanVehicleBeacon(ctx context.Context, vin string) (Advertisement, error) {
+	mu.Lock()
+	defer mu.Unlock()
+
+	if err := initDevice(); err != nil {
+		return nil, err
+	}
+
+	a, err := scanVehicleBeacon(ctx, VehicleLocalName(vin))
+	if err != nil {
+		return nil, fmt.Errorf("ble: failed to scan for %s: %s", vin, err)
+	}
+	return a, nil
+}
+
+func scanVehicleBeacon(ctx context.Context, localName string) (Advertisement, error) {
+	var err error
+	ctx2, cancel := context.WithCancel(ctx)
+	defer cancel()
+
+	ch := make(chan Advertisement, 1)
+	fn := func(a Advertisement) {
+		if a.LocalName() != localName {
+			return
+		}
+		select {
+		case ch <- a:
+			cancel() // Notify device.Scan() that we found a match
+		case <-ctx2.Done():
+			// Another goroutine already found a matching advertisement. We need to return so that
+			// the MacOS implementation of device.Scan(...) unblocks.
+		}
+	}
+
+	if err = device.Scan(ctx2, false, fn); !errors.Is(err, context.Canceled) {
+		// If ctx rather than ctx2 was canceled, we'll pick that error up below. This is a bit
+		// hacky, but unfortunately device.Scan() _always_ returns an error on MacOS because it does
+		// not terminate until the provided context is canceled.
+		return nil, err
+	}
+
+	select {
+	case a, ok := <-ch:
+		if !ok {
+			// This should never happen, but just in case
+			return nil, fmt.Errorf("scan channel closed")
+		}
+		return a, nil
+	case <-ctx.Done():
+		return nil, ctx.Err()
+	}
+}
+
 func NewConnection(ctx context.Context, vin string) (*Connection, error) {
+	return NewConnectionToBleTarget(ctx, vin, nil)
+}
+
+func NewConnectionToBleTarget(ctx context.Context, vin string, target Advertisement) (*Connection, error) {
 	var lastError error
 	for {
-		conn, err := tryToConnect(ctx, vin)
+		conn, retry, err := tryToConnect(ctx, vin, target)
 		if err == nil {
 			return conn, nil
 		}
-		if strings.Contains(err.Error(), "operation not permitted") {
+		if !retry || strings.Contains(err.Error(), "operation not permitted") {
 			return nil, err
 		}
 		log.Warning("BLE connection attempt failed: %s", err)
@@ -146,61 +230,51 @@ func NewConnection(ctx context.Context, vin string) (*Connection, error) {
 	}
 }
 
-func tryToConnect(ctx context.Context, vin string) (*Connection, error) {
+func tryToConnect(ctx context.Context, vin string, target Advertisement) (*Connection, bool, error) {
 	var err error
-	// We don't want concurrent calls to NewConnection that would defeat
-	// the point of reusing the existing BLE device. Note that this is not
-	// an issue on MacOS, but multiple calls to newDevice() on Linux leads to failures.
 	mu.Lock()
 	defer mu.Unlock()
 
-	if device != nil {
-		log.Debug("Reusing existing BLE device")
-	} else {
-		log.Debug("Creating new BLE device")
-		device, err = newDevice()
-		if err != nil {
-			return nil, fmt.Errorf("failed to find a BLE device: %s", err)
-		}
-		ble.SetDefaultDevice(device)
+	if err = initDevice(); err != nil {
+		return nil, false, err
 	}
 
-	vinBytes := []byte(vin)
-	digest := sha1.Sum(vinBytes)
+	localName := VehicleLocalName(vin)
 
-	localName := fmt.Sprintf("S%02xC", digest[:8])
-	log.Debug("Searching for BLE beacon %s...", localName)
-	canConnect := false
-	filter := func(adv ble.Advertisement) bool {
-		ln := adv.LocalName()
-		if ln != localName {
-			return false
+	if target == nil {
+		target, err = scanVehicleBeacon(ctx, localName)
+		if err != nil {
+			return nil, true, fmt.Errorf("ble: failed to scan for %s: %s", vin, err)
 		}
-		canConnect = adv.Connectable()
-		return true
 	}
 
-	client, err := ble.Connect(ctx, filter)
-	if err != nil {
-		return nil, fmt.Errorf("failed to find BLE beacon for %s (%s): %s", vin, localName, err)
+	if target.LocalName() != localName {
+		return nil, false, fmt.Errorf("ble: beacon with unexpected local name: '%s'", target.LocalName())
 	}
 
-	if !canConnect {
-		return nil, ErrMaxConnectionsExceeded
+	if !target.Connectable() {
+		return nil, false, ErrMaxConnectionsExceeded
+	}
+
+	log.Debug("Dialing to %s (%s)...", target.Addr(), localName)
+
+	client, err := ble.Dial(ctx, target.Addr())
+	if err != nil {
+		return nil, true, fmt.Errorf("ble: failed to dial for %s (%s): %s", vin, localName, err)
 	}
 
-	log.Debug("Connecting to BLE beacon %s...", client.Addr())
+	log.Debug("Discovering services %s...", client.Addr())
 	services, err := client.DiscoverServices([]ble.UUID{vehicleServiceUUID})
 	if err != nil {
-		return nil, fmt.Errorf("ble: failed to enumerate device services: %s", err)
+		return nil, true, fmt.Errorf("ble: failed to enumerate device services: %s", err)
 	}
 	if len(services) == 0 {
-		return nil, fmt.Errorf("ble: failed to discover service")
+		return nil, true, fmt.Errorf("ble: failed to discover service")
 	}
 
 	characteristics, err := client.DiscoverCharacteristics([]ble.UUID{toVehicleUUID, fromVehicleUUID}, services[0])
 	if err != nil {
-		return nil, fmt.Errorf("ble: failed to discover service characteristics: %s", err)
+		return nil, true, fmt.Errorf("ble: failed to discover service characteristics: %s", err)
 	}
 
 	conn := Connection{
@@ -215,15 +289,15 @@ func tryToConnect(ctx context.Context, vin string) (*Connection, error) {
 			conn.rxChar = characteristic
 		}
 		if _, err := client.DiscoverDescriptors(nil, characteristic); err != nil {
-			return nil, fmt.Errorf("ble: couldn't fetch descriptors: %s", err)
+			return nil, true, fmt.Errorf("ble: couldn't fetch descriptors: %s", err)
 		}
 	}
 	if conn.txChar == nil || conn.rxChar == nil {
-		return nil, fmt.Errorf("ble: failed to find required characteristics")
+		return nil, true, fmt.Errorf("ble: failed to find required characteristics")
 	}
 	if err := client.Subscribe(conn.rxChar, true, conn.rx); err != nil {
-		return nil, fmt.Errorf("ble: failed to subscribe to RX: %s", err)
+		return nil, true, fmt.Errorf("ble: failed to subscribe to RX: %s", err)
 	}
 	log.Info("Connected to vehicle BLE")
-	return &conn, nil
+	return &conn, false, nil
 }