-
Notifications
You must be signed in to change notification settings - Fork 4
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
base: main
Are you sure you want to change the base?
Changes from 17 commits
f31130c
68694d9
7d101b0
6fa786f
c5c1bc6
4b5fcc6
6edd5cd
02dd56f
80d0929
6ac1d56
0356d47
f362c74
9a47333
5923659
16516e3
74bf5c3
cf43f28
12afb28
8400c94
9377326
0aa9149
ca55251
325f9b7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
@@ -0,0 +1,171 @@ | ||||
// 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" | ||||
"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 { | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lets reuse agent/subsystems/provisioning/definitions.go Line 257 in 90b7f59
These types will be the same in the BLE and the Hotspot flow There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved into |
||||
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 { | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you can re-use this type agent/subsystems/provisioning/definitions.go Line 112 in 90b7f59
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved into |
||||
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. | ||||
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. | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Logging is included in the implementation of |
||||
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. | ||||
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 = retryCallbackOnExpectedError( | ||||
ctx, bwp.readSsid, &emptyBluetoothCharacteristicError{}, "failed to read ssid", | ||||
); ssidErr != nil { | ||||
cancel() | ||||
} | ||||
}, | ||||
wg.Done, | ||||
) | ||||
utils.ManagedGo( | ||||
func() { | ||||
if psk, pskErr = retryCallbackOnExpectedError( | ||||
ctx, bwp.readPsk, &emptyBluetoothCharacteristicError{}, "failed to read psk", | ||||
); pskErr != nil { | ||||
cancel() | ||||
} | ||||
|
||||
}, | ||||
wg.Done, | ||||
) | ||||
} | ||||
if requiresCloudCredentials { | ||||
wg.Add(2) | ||||
utils.ManagedGo( | ||||
func() { | ||||
if robotPartKeyID, robotPartKeyIDErr = retryCallbackOnExpectedError( | ||||
ctx, bwp.readRobotPartKeyID, &emptyBluetoothCharacteristicError{}, "failed to read robot part key ID", | ||||
); robotPartKeyIDErr != nil { | ||||
cancel() | ||||
} | ||||
}, | ||||
wg.Done, | ||||
) | ||||
utils.ManagedGo( | ||||
func() { | ||||
if robotPartKey, robotPartKeyErr = retryCallbackOnExpectedError( | ||||
ctx, bwp.readRobotPartKey, &emptyBluetoothCharacteristicError{}, "failed to read robot part key", | ||||
); robotPartKeyErr != nil { | ||||
cancel() | ||||
} | ||||
}, | ||||
wg.Done, | ||||
) | ||||
} | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this pattern feels odd to me - why are we starting 4 goroutines that's each calling 1 function in a loop? Something useful for context would be to know on a high level how each of the functions will work There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To simplify handling asynchronous Bluetooth reads, I created a function that calls Rather than leaving this logic to the caller, which could be error-prone, this function ensures all values are retrieved before returning. And in the error case, it cancels context for all. Each function checks an in-memory store for existing data. The requiresWiFiCredentials and requiresCloudCredentials booleans add flexibility, allowing callers to request only WiFi credentials, cloud credentials, or both as needed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm looking at https://github.com/viamrobotics/agent/pull/68/files so let me know if I'm missing something. It seems possible to only have 1 goroutine that calls readSsid, readPsk, readRobotPartKeyID, and readRobotPartKey sequentially and then retry every second, is there a reason why not? |
||||
wg.Wait() | ||||
return &Credentials{ | ||||
Ssid: ssid, Psk: psk, RobotPartKeyID: robotPartKeyID, RobotPartKey: robotPartKey, | ||||
}, multierr.Combine(ssidErr, pskErr, robotPartKeyIDErr, robotPartKeyErr) | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can use |
||||
} | ||||
|
||||
/** 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": | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pls no. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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,43 @@ | ||
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) | ||
} | ||
|
||
// retryCallbackOnExpectedError retries the provided callback to at one second intervals as long as an expected error is thrown. | ||
func retryCallbackOnExpectedError( | ||
ctx context.Context, fn func() (string, error), expectedErr 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 { | ||
if errors.As(err, &expectedErr) { | ||
continue | ||
} | ||
return "", errors.WithMessagef(err, "%s", description) | ||
} | ||
return v, nil | ||
} | ||
} |
There was a problem hiding this comment.
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 haserrors.WithMessage(err, "some wrapping message")
anderrors.Errorf("message with arg %s", arg)
(which do not exist in the standard "errors" package).There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.