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

BLE: allow client to override write with/without response #159

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 5 additions & 3 deletions Documentation/BluetoothLE.md
Original file line number Diff line number Diff line change
Expand Up @@ -274,11 +274,13 @@ controls which of these modes shall be used for a particular write.
the write was received without error, and Scratch Link's response to the client shall report any error reported by the
BLE peripheral. If the peripheral reports an error, that error shall be forwarded to the client as an error response
to the "write" request.
- If false or absent, Scratch Link shall write without response. That is, Scratch Link shall make a [best-effort
- If false, Scratch Link shall write without response. That is, Scratch Link shall make a [best-effort
delivery](https://en.wikipedia.org/wiki/Best-effort_delivery) attempt then report success. There is no way for the
peripheral to report an error in this mode.
- If the peripheral or characteristic does not support writing with response, Scratch Link may choose to write without
response even when the "withResponse" flag is true.
- If absent, Scratch Link shall check if the characteristic appears to support writing without response. If so,
Scratch Link shall write without response. Otherwise, Scratch shall write with response.

Generally, writing without response is significantly faster.

On success, Scratch Link's **response** shall contain the number of bytes written, which may differ from the number of
characters in the string value of the initiating request's "message" property:
Expand Down
4 changes: 4 additions & 0 deletions Documentation/NetworkProtocol.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ This version number shall follow the Semantic Versioning specification, found he

### Version History

- Version 1.3:
- Bluetooth LE:
- Alter Scratch Link's handling of the `withResponse` flag on a `write` request. The flag now overrides Scratch
Link's detection of GATT characteristic flags.
- Version 1.2:
- Add `manufacturerData` filtering for BLE discovery.
- Add common `getVersion` method.
Expand Down
2 changes: 1 addition & 1 deletion Windows/scratch-link/BLESession.cs
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ private async Task<JToken> Write(JObject parameters)
{
var buffer = EncodingHelpers.DecodeBuffer(parameters);
var endpoint = await GetEndpoint("write request", parameters, GattHelpers.BlockListStatus.ExcludeWrites);
var withResponse = (parameters["withResponse"]?.ToObject<bool>() ?? false) ||
var withResponse = parameters["withResponse"]?.ToObject<bool>() ??
!endpoint.CharacteristicProperties.HasFlag(GattCharacteristicProperties.WriteWithoutResponse);
Copy link
Contributor

Choose a reason for hiding this comment

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

Set to a variable "the characteristic thinks we MUST write WITH response"

Copy link
Contributor

Choose a reason for hiding this comment

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

It might be good to say that we explicitly expect an error if the gatt characteristic does not have property write with response (because then we're trying to write and we're not allowed to write at all)


var result = await endpoint.WriteValueWithResultAsync(buffer.AsBuffer(),
Expand Down
7 changes: 5 additions & 2 deletions macOS/Sources/scratch-link/BLESession.swift
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ class BLESession: Session, SwiftCBCentralManagerDelegate, SwiftCBPeripheralDeleg

func write(withParams params: [String: Any], completion: @escaping JSONRPCCompletionHandler) throws {
let buffer = try EncodingHelpers.decodeBuffer(fromJSON: params)
let withResponse = params["withResponse"] as? Bool ?? false
let withResponse = params["withResponse"] as? Bool

getEndpoint(for: "write request", withParams: params, blockedBy: .ExcludeWrites) { endpoint, error in
if let error = error {
Expand All @@ -297,7 +297,10 @@ class BLESession: Session, SwiftCBCentralManagerDelegate, SwiftCBPeripheralDeleg
return
}

let writeType = (withResponse || !endpoint.properties.contains(.writeWithoutResponse)) ?
// If the client specified a write type, honor that.
Copy link
Contributor

Choose a reason for hiding this comment

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

Add this comment to c# as well

// Otherwise, if the characteristic claims to support writing without response, do that.
// Otherwise, write with response.
let writeType = (withResponse ?? !endpoint.properties.contains(.writeWithoutResponse)) ?
Copy link
Contributor

Choose a reason for hiding this comment

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

withResponse = withResponse ?? !endpoint.properties.contains(.writeWithoutResponse));

let writeType = withResponse ? x : y

might be more clear?

CBCharacteristicWriteType.withResponse : CBCharacteristicWriteType.withoutResponse
peripheral.writeValue(buffer, for: endpoint, type: writeType)
completion(buffer.count, nil)
Expand Down