Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[APP-7635]: Define type for managing bluetooth WiFi provisioning. #62

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
f31130c
Add bluetooth wifi provisioner interface only (still need to add nest…
maxhorowitz Feb 13, 2025
68694d9
Add bluetooth service (low-level) interface for abstracting away OS/f…
maxhorowitz Feb 14, 2025
7d101b0
WIP
maxhorowitz Feb 14, 2025
6fa786f
Remove example.go file.
maxhorowitz Feb 14, 2025
c5c1bc6
Improve inline commenting.
maxhorowitz Feb 14, 2025
4b5fcc6
Update subsystems/provisioning/bluetooth/bluetooth_service.go
maxhorowitz Feb 14, 2025
6edd5cd
Update subsystems/provisioning/bluetooth/bluetooth_service.go
maxhorowitz Feb 14, 2025
02dd56f
Remove both interfaces and add inline TODOs where Linux functionality…
maxhorowitz Feb 18, 2025
80d0929
Remove error check from enableAutoAcceptPairRequest due to asynch nat…
maxhorowitz Feb 18, 2025
6ac1d56
Use standard library errors and import custom errors package with pre…
maxhorowitz Feb 18, 2025
0356d47
Pass context one layer lower in call stack to properly protect agains…
maxhorowitz Feb 18, 2025
f362c74
Remove unused helper method for custom error type.
maxhorowitz Feb 18, 2025
9a47333
Add context cancellation in case of error in WaitForCredentials metho…
maxhorowitz Feb 18, 2025
5923659
Update subsystems/provisioning/bluetooth/bluetooth_wifi_provisioner_u…
maxhorowitz Feb 18, 2025
16516e3
Make retryCallbackOnExpectedError fn much more general.
maxhorowitz Feb 18, 2025
74bf5c3
Update subsystems/provisioning/bluetooth/bluetooth_wifi_provisioner.go
maxhorowitz Feb 18, 2025
cf43f28
Update subsystems/provisioning/bluetooth/bluetooth_wifi_provisioner.go
maxhorowitz Feb 18, 2025
12afb28
Update error message TODOs to be more specific.
maxhorowitz Feb 18, 2025
8400c94
Move bluetooth wifi provisioner file into provisioning package and re…
maxhorowitz Feb 18, 2025
9377326
Remove nested goroutines for single goroutine.
maxhorowitz Feb 20, 2025
0aa9149
Removed some of the unneccessary layers of abstraction, and added an …
maxhorowitz Feb 20, 2025
ca55251
Add interface (for bluetoothService) to conceal all fields and helper…
maxhorowitz Feb 20, 2025
325f9b7
Add nolint tags where necessary to get the code to pass lint (most is…
maxhorowitz Feb 20, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ require (
github.com/pkg/errors v0.9.1
github.com/sergeymakinen/go-systemdconf/v2 v2.0.2
github.com/ulikunitz/xz v0.5.12
go.uber.org/multierr v1.11.0
go.uber.org/zap v1.27.0
go.viam.com/api v0.1.357
go.viam.com/rdk v0.51.0
Expand Down Expand Up @@ -79,7 +80,6 @@ require (
go.mongodb.org/mongo-driver v1.17.1 // indirect
go.opencensus.io v0.24.0 // indirect
go.uber.org/goleak v1.3.0 // indirect
go.uber.org/multierr v1.11.0 // indirect
golang.org/x/crypto v0.28.0 // indirect
golang.org/x/mod v0.21.0 // indirect
golang.org/x/net v0.30.0 // indirect
Expand Down
169 changes: 169 additions & 0 deletions subsystems/provisioning/bluetooth/bluetooth_wifi_provisioner.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
// Package ble contains an interface for using bluetooth-low-energy to retrieve WiFi and robot part credentials for an unprovisioned Agent.
package ble

import (
"context"
"encoding/json"
"runtime"
"sync"

"errors"

errw "github.com/pkg/errors"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copied this (including the errw prefix) from elsewhere in the Agent. The "github.com/pkg/errors" package has errors.WithMessage(err, "some wrapping message") and errors.Errorf("message with arg %s", arg) (which do not exist in the standard "errors" package).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume that using errw is just so we can migrate old code without too much hassle, but for new code you can use fmt.Errorf

// An easy way to create wrapped errors is to call [fmt.Errorf] and apply
// the %w verb to the error argument:
//
//	wrapsErr := fmt.Errorf("... %w ...", ..., err, ...)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome. Didn't know it was doable that way. Will change to this, thanks.

"go.uber.org/multierr"
"go.viam.com/rdk/logging"
"go.viam.com/utils"
)

// Credentials represent the minimum required information needed to provision a Viam Agent.
type Credentials struct {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets reuse

type userInput struct {

These types will be the same in the BLE and the Hotspot flow

Copy link
Member Author

@maxhorowitz maxhorowitz Feb 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can use that type but will have to move it out of the provisioning package and into an isolated package where both can import it (to get around an import cycle). Does that sound like something you would want here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved into provisioning and reused this type.

Ssid string
Psk string
RobotPartKeyID string
RobotPartKey string
}

// AvailableWiFiNetworks represent the networks that the device has detected (and which may be available for connection).
type AvailableWiFiNetworks struct {
Networks []*struct {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can re-use this type

type NetworkInfo struct {

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as the above with regards to import cycles. Can use, but would need to pull that out. Otherwise, maybe we should just be moving this code into the existing provisioning package as its own file where it can also access those values?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved into provisioning and reused this type.

Ssid string `json:"ssid"`
Strength float64 `json:"strength"` // Inclusive range [0.0, 1.0], represents the % strength of a WiFi network.
RequiresPsk bool `json:"requires_psk"`
} `json:"networks"`
}

func (awns *AvailableWiFiNetworks) ToBytes() ([]byte, error) {
return json.Marshal(awns)
}

// bluetoothWiFiProvisioner provides an interface for managing BLE (bluetooth-low-energy) peripheral advertisement on Linux.

Check failure on line 39 in subsystems/provisioning/bluetooth/bluetooth_wifi_provisioner.go

View workflow job for this annotation

GitHub Actions / Test lint and build

ST1021: comment on exported type BluetoothWiFiProvisioner should be of the form "BluetoothWiFiProvisioner ..." (with optional leading article) (stylecheck)
type BluetoothWiFiProvisioner struct{}

// Start begins advertising a bluetooth service that acccepts WiFi and Viam cloud config credentials.
func (bwp *BluetoothWiFiProvisioner) Start(ctx context.Context) error {
if err := bwp.startAdvertisingBLE(ctx); err != nil {
return err
}
bwp.enableAutoAcceptPairRequest() // Async goroutine (hence no error check) which auto-accepts pair requests on this device.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this mean we won't provide helpful logging to the users? or that part will be implemented in enableAutoAcceptPairRequest?

Copy link
Member Author

@maxhorowitz maxhorowitz Feb 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logging is included in the implementation of enableAutoAcceptPairRequest. The stacked PR containing its impl is here: #69.

return nil
}

// Stop stops advertising a bluetooth service which (when enabled) accepts WiFi and Viam cloud config credentials.
func (bwp *BluetoothWiFiProvisioner) Stop() error {
return bwp.stopAdvertisingBLE()
}

// Update updates the list of networks that are advertised via bluetooth as available.

Check failure on line 56 in subsystems/provisioning/bluetooth/bluetooth_wifi_provisioner.go

View workflow job for this annotation

GitHub Actions / Test lint and build

ST1020: comment on exported method RefreshAvailableNetworks should be of the form "RefreshAvailableNetworks ..." (stylecheck)
func (bwp *BluetoothWiFiProvisioner) RefreshAvailableNetworks(ctx context.Context, awns *AvailableWiFiNetworks) error {
return bwp.writeAvailableNetworks(ctx, awns)
}

// WaitForCredentials returns credentials, the minimum required information to provision a robot and/or its WiFi.
func (bwp *BluetoothWiFiProvisioner) WaitForCredentials(ctx context.Context, requiresCloudCredentials bool, requiresWiFiCredentials bool) (*Credentials, error) {
ctx, cancel := context.WithCancel(ctx)
defer cancel()

if !requiresWiFiCredentials && !requiresCloudCredentials {
return nil, errors.New("should be waiting for either cloud credentials or WiFi credentials")
}
var ssid, psk, robotPartKeyID, robotPartKey string
var ssidErr, pskErr, robotPartKeyIDErr, robotPartKeyErr error
wg := &sync.WaitGroup{}
if requiresWiFiCredentials {
wg.Add(2)
utils.ManagedGo(
func() {
if ssid, ssidErr = retryCallbackOnEmptyCharacteristicError(
ctx, bwp.readSsid, "ssid",
); ssidErr != nil {
cancel()
}
},
wg.Done,
)
utils.ManagedGo(
func() {
if psk, pskErr = retryCallbackOnEmptyCharacteristicError(
ctx, bwp.readPsk, "psk",
); pskErr != nil {
cancel()
}

},
wg.Done,
)
}
if requiresCloudCredentials {
wg.Add(2)
utils.ManagedGo(
func() {
if robotPartKeyID, robotPartKeyIDErr = retryCallbackOnEmptyCharacteristicError(
ctx, bwp.readRobotPartKeyID, "robot part key ID",
); robotPartKeyIDErr != nil {
cancel()
}
},
wg.Done,
)
utils.ManagedGo(
func() {
if robotPartKey, robotPartKeyErr = retryCallbackOnEmptyCharacteristicError(
ctx, bwp.readRobotPartKey, "robot part key",
); robotPartKeyErr != nil {
cancel()
}
},
wg.Done,
)
}
wg.Wait()
return &Credentials{
Ssid: ssid, Psk: psk, RobotPartKeyID: robotPartKeyID, RobotPartKey: robotPartKey,
}, multierr.Combine(ssidErr, pskErr, robotPartKeyIDErr, robotPartKeyErr)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can use errors.Join

}

/** Unexported helper methods for low-level system calls and read/write requests to/from bluetooth characteristics **/

func (bwp *BluetoothWiFiProvisioner) startAdvertisingBLE(ctx context.Context) error {
return errors.New("TODO APP-7644: Add Linux-specific bluetooth calls for automatic pairing and read/write to BLE characteristics")
}

func (bwp *BluetoothWiFiProvisioner) stopAdvertisingBLE() error {
return errors.New("TODO APP-7644: Add Linux-specific bluetooth calls for automatic pairing and read/write to BLE characteristics")
}

func (bwp *BluetoothWiFiProvisioner) enableAutoAcceptPairRequest() {}

func (bwp *BluetoothWiFiProvisioner) writeAvailableNetworks(ctx context.Context, networks *AvailableWiFiNetworks) error {
return errors.New("TODO APP-7644: Add Linux-specific bluetooth calls for automatic pairing and read/write to BLE characteristics")
}

func (bwp *BluetoothWiFiProvisioner) readSsid() (string, error) {
return "", errors.New("TODO APP-7644: Add Linux-specific bluetooth calls for automatic pairing and read/write to BLE characteristics")
}

func (bwp *BluetoothWiFiProvisioner) readPsk() (string, error) {
return "", errors.New("TODO APP-7644: Add Linux-specific bluetooth calls for automatic pairing and read/write to BLE characteristics")
}

func (bwp *BluetoothWiFiProvisioner) readRobotPartKeyID() (string, error) {
return "", errors.New("TODO APP-7644: Add Linux-specific bluetooth calls for automatic pairing and read/write to BLE characteristics")
}

func (bwp *BluetoothWiFiProvisioner) readRobotPartKey() (string, error) {
return "", errors.New("TODO APP-7644: Add Linux-specific bluetooth calls for automatic pairing and read/write to BLE characteristics")
}

// NewBluetoothWiFiProvisioner returns a service which accepts credentials over bluetooth to provision a robot and its WiFi connection.
func NewBluetoothWiFiProvisioner(ctx context.Context, logger logging.Logger, name string) (*BluetoothWiFiProvisioner, error) {
switch os := runtime.GOOS; os {
case "linux":
fallthrough
case "windows":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pls no.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😅

fallthrough
case "darwin":
fallthrough
default:
return nil, errw.Errorf("failed to set up bluetooth-low-energy peripheral, %s is not yet supported", os)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package ble

import (
"context"
"fmt"
"time"

"github.com/pkg/errors"
)

// emptyBluetoothCharacteristicError represents the error which is raised when we attempt to read from an empty BLE characteristic.
type emptyBluetoothCharacteristicError struct {
missingValue string
}

func (e *emptyBluetoothCharacteristicError) Error() string {
return fmt.Sprintf("no value has been written to BLE characteristic for %s", e.missingValue)
}

// waitForBLE is used to check for the existence of a new value in a BLE characteristic.
func retryCallbackOnEmptyCharacteristicError(
ctx context.Context, fn func() (string, error), description string,
) (string, error) {
for {
if ctx.Err() != nil {
return "", ctx.Err()
}
select {
case <-ctx.Done():
return "", ctx.Err()
default:
time.Sleep(time.Second)
}
v, err := fn()
if err != nil {
var errBLECharNoValue *emptyBluetoothCharacteristicError
if errors.As(err, &errBLECharNoValue) {
continue
}
return "", errors.WithMessagef(err, "failed to read %s", description)
}
return v, nil
}
}
Loading