Remove auto reconnect, ConnectionStatus.Reconnecting, and discarding of device on disconnect#85
Conversation
|
This seems to break iOS pairing and then flashing. Investigating... Update: should be fixed by 1fff6b2 |
…onnection failure
Deploying microbit-connection with
|
| Latest commit: |
205fdb6
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://307abdf1.microbit-connection.pages.dev |
| Branch Preview URL: | https://remove-auto-reconnect.microbit-connection.pages.dev |
microbit-grace
changed the title
ConnectionStatus.Reconnecting, and discarding of device on disconnect
| this.onPeriodicMessageReceived = undefined; | ||
| reject(new Error("Failed to receive data from remote micro:bit")); | ||
| }, 500); | ||
| }, connectTimeoutDuration); |
There was a problem hiding this comment.
Increased connection timeout from 500ms to 10_000ms to match bluetooth (https://github.com/microbit-foundation/microbit-connection/blob/remove-auto-reconnect/lib/bluetooth-device-wrapper.ts#L45)
lib/bluetooth-device-wrapper.ts
Outdated
| async disconnect(): Promise<void> { | ||
| return this.disconnectInternal(true); | ||
| return this.disconnectInternal(); | ||
| } | ||
|
|
||
| private async disconnectInternal(userTriggered: boolean): Promise<void> { | ||
| this.logging.log( | ||
| `Bluetooth disconnect ${userTriggered ? "(user triggered)" : "(programmatic)"}`, | ||
| ); | ||
| this.duringExplicitConnectDisconnect++; | ||
| private async disconnectInternal(): Promise<void> { | ||
| this.logging.log("Bluetooth disconnect"); |
There was a problem hiding this comment.
I guess we can collapse these now.
There was a problem hiding this comment.
Found that knowing whether the disconnect is occurring programatically vs user triggered is quite useful. Not sure why I removed that in the first place. Have recovered it here -> 713edeb
lib/bluetooth-device-wrapper.ts
Outdated
| e, | ||
| ); | ||
| } | ||
| this.logging.log("Bluetooth disconnect"); |
There was a problem hiding this comment.
Feels like we should log something different from the disconnectInternal logging to help distinguish.
There was a problem hiding this comment.
Your original thought that it is an redundant extra logging during our discussion was somewhat correct. It does log twice (duplicating above log) when we disconnect explicitly by user/programmatically, but only logs once when the disconnect is implicit (error-related).
I have removed this logging to avoid duplication (e141be7). If there is an error-related disconnect, nothing will get logged as it is unexpected
lib/usb-radio-bridge.ts
Outdated
| connectTimeoutDuration | ||
| ) { | ||
| await this.restartConnection(); | ||
| this.callbacks.onBeforeConnectionLostDispose(); |
There was a problem hiding this comment.
As discussed please just ponder whether this could have a clearer name.
For the scenario where user disconnects remote for a few seconds, switches tab for a few second, and returns to the tab with the connection. Without keeping serial session open, the radio bridge connection does not reconnect.
|
Can you check if we still need ignoreDelegateStatus ? |
Good spot, looks like it is no longer ever set as |
2 participants
ConnectionStatus.Reconnectingstatus in favour of the consuming app handling reconnections where necessary.clearDevice. So unless the app callsclearDevice, we assume the same device is used for the connection.ignoreDelegateStatus