Proactive disconnect reporting #85
Conversation
| @@ -187,7 +192,7 @@ class BTSession: Session, IOBluetoothRFCOMMChannelDelegate, IOBluetoothDeviceInq | |||
|
|
|||
| func deviceInquiryDeviceNameUpdated( | |||
| _ sender: IOBluetoothDeviceInquiry!, device: IOBluetoothDevice!, devicesRemaining: UInt32) { | |||
| print("name updated: \(device.name)") | |||
| print("name updated: \(String(describing: device.name))") | |||
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
| new JProperty("id", responseId), | ||
| error == null ? new JProperty("result", result) : new JProperty("error", JObject.FromObject(error)) | ||
| ); | ||
| var response = MakeResponse(responseId, result, error); |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
|
I just made a new build with this branch and tried the following:
Expected
Actual
Was this case supposed to be addressed with this branch's change? Scratch Link Build No. : |
|
@evhan55 Interesting. I mostly tested with the micro:bit and EV3 so maybe the WeDo 2's behavior is different somehow. I wouldn't expect so, but... maybe? Anyway, I'll see if I can repro. Thanks! Just to clarify expectations: I would expect Scratch Link to send an error but not actually disconnect the socket. I would expect the extension to disconnect the socket in response to the error. |
|
The error that it sends back, if it's not in response to a write/read, then is it something that would be documented and can be handled in |
|
Based on discussion with @evhan55 I'm going to try a slightly different approach... to be continued! |
|
Trying build version: Still doesn't seem to auto-close the socket on the Chrome side of things. Question: how long should I be waiting for the auto-close? |
|
Hmm... it should auto-close more or less immediately. In my experience it happens in less than a second. |
|
Yes! Mac OS Mojave with WeDo 2. |
|
I re-tested with a fresh build from this branch:
Expected: Scratch VM gets the web socket closed error and dispatches error alerts Actual: Nothing happens until I click a Scratch Link Version: |
|
@evhan55: OK, now that I understand what you're seeing I see the same thing. Unfortunately I don't currently see a way to catch this. The OS seems to notice that the peripheral has gone away, but none of the APIs that I'm using seem to report it at the Swift / CoreBluetooth level. It does seem to report an error and disconnect reliably if your computer falls asleep or if the user disables Bluetooth, though. |
|
In the interest of getting a new build into MAS review I'm going to ahead and merge this for now. I filed #87 to track the issue from the conversation above. |
Review round 2:
This new version is still sending the error notification to the client, but also closes the web socket. This means that the overall behavior is now guaranteed correct from a "1 session = 1 socket = 1 peripheral" point of view without the client doing anything special for this situation.
Review round 1:
This change ensures that the client (extension / extension framework) is notified about a connection problem as soon as possible. Previously, in many cases, the extension didn't hear about a connection problem until it tried to perform a read or write on the peripheral, which depending on the extension may happen only rarely.
In general this is handled by sending an error notification to the client. To make this easier, I pulled some of the response construction code out into a function that could be called both by the regular response path and the error notification path.
Note that BLE on Windows was already working well, so changes were only necessary for BT on Windows and both BLE and BT on Mac.