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

Nordic UART service bug: should use notifications not indications #259

Open
bluetooth-mdw opened this Issue Jan 12, 2017 · 34 comments

Comments

Projects
None yet
9 participants
@bluetooth-mdw
Contributor

bluetooth-mdw commented Jan 12, 2017

Nordic's nRF Toolbox application includes a UART client. It will not work with the UART service on micro:bit however. It gives an error saying the required services are not on the micro:bit. In fact the correct service is there but the nRF Toolbox application goes further and checks the UUIDs of the characteristics in the service and their properties. The characteristic with UUID 6E400002B5A3F393E0A9E50E24DCCA9E which we call the TX characteristic, uses Indications to transmit data from the micro:bit to the connected client. It should in fact use Notifications and this is why the Nordic application is rejecting the device. I got this from Nordic support by the way.

I don't know the background to the implementation of the UART service on micro:bit. I'm 99.99% certain I commented on the use of Indications rather than Notifdications to @jamesadevine at some point but forget the answer. It may be there's some rogue documentation out there somewhere.

Either way, this needs fixing. Happy to take care of it as it's a very small change I think.

@finneyj

This comment has been minimized.

Show comment
Hide comment
@finneyj

finneyj Jan 12, 2017

Contributor

thanks @bluetooth-mdw - awesome analysis as ever!

Aligning with Nordic's service would be a good idea I think, and notifications make more sense in my mind anyway i think. Unless anyone out there can think of any compelling reasons to the contrary, I'd suggest we change it.

@martinwork - Any significant implications in changing this for your app?

Contributor

finneyj commented Jan 12, 2017

thanks @bluetooth-mdw - awesome analysis as ever!

Aligning with Nordic's service would be a good idea I think, and notifications make more sense in my mind anyway i think. Unless anyone out there can think of any compelling reasons to the contrary, I'd suggest we change it.

@martinwork - Any significant implications in changing this for your app?

@bluetooth-mdw

This comment has been minimized.

Show comment
Hide comment
@bluetooth-mdw

bluetooth-mdw Jan 12, 2017

Contributor

There's more to this. Nordic have done some digging and come back saying that they have some inconsistencies in their own documentation and code. This is doubtless where our own issues have come from. They've logged a bug in their issue tracker. Right now it's believed the right spec is:

TX characteristic

UUID: 0x0002
Full UUID: 6E400002-B5A3-F393-E0A9-E50E24DCCA9E
Properties: WRITE, WRITE WITOUT RESPONSE
RX characteristic

UUID: 0x0003
Full UUID: 6E400003-B5A3-F393-E0A9-E50E24DCCA9E
Properties: NOTIFICATION

which would also mean we have TX and RX the wrong way around. Confusing, huh?!

I think we should wait for the dust to settle and once we're confident the final word from Nordic has been heard, only then move to change it in the DAL.

Contributor

bluetooth-mdw commented Jan 12, 2017

There's more to this. Nordic have done some digging and come back saying that they have some inconsistencies in their own documentation and code. This is doubtless where our own issues have come from. They've logged a bug in their issue tracker. Right now it's believed the right spec is:

TX characteristic

UUID: 0x0002
Full UUID: 6E400002-B5A3-F393-E0A9-E50E24DCCA9E
Properties: WRITE, WRITE WITOUT RESPONSE
RX characteristic

UUID: 0x0003
Full UUID: 6E400003-B5A3-F393-E0A9-E50E24DCCA9E
Properties: NOTIFICATION

which would also mean we have TX and RX the wrong way around. Confusing, huh?!

I think we should wait for the dust to settle and once we're confident the final word from Nordic has been heard, only then move to change it in the DAL.

@finneyj

This comment has been minimized.

Show comment
Hide comment
@finneyj

finneyj Jan 12, 2017

Contributor

Yep.

I remember talking to them about which point of view TX and RX is, and there was definitely some confusion there. Happy to let the dust settle.

Contributor

finneyj commented Jan 12, 2017

Yep.

I remember talking to them about which point of view TX and RX is, and there was definitely some confusion there. Happy to let the dust settle.

@bluetooth-mdw

This comment has been minimized.

Show comment
Hide comment
@bluetooth-mdw

bluetooth-mdw Jan 12, 2017

Contributor

Nordic just said "This is definitely the final word". They seem to have thoroughly investigated. So.... I think we can proceed when it suits us now.

Contributor

bluetooth-mdw commented Jan 12, 2017

Nordic just said "This is definitely the final word". They seem to have thoroughly investigated. So.... I think we can proceed when it suits us now.

@finneyj

This comment has been minimized.

Show comment
Hide comment
@finneyj

finneyj Jan 12, 2017

Contributor

OK - well we are using their UUIDs, so we should meet their spec. Let's flip the TX/RX 'lines' as of the next release. No sense prolonging incompatibilities...

Contributor

finneyj commented Jan 12, 2017

OK - well we are using their UUIDs, so we should meet their spec. Let's flip the TX/RX 'lines' as of the next release. No sense prolonging incompatibilities...

@bluetooth-mdw

This comment has been minimized.

Show comment
Hide comment
@bluetooth-mdw

bluetooth-mdw Jan 12, 2017

Contributor

OK. I've penciled this in for Saturday morning.

Contributor

bluetooth-mdw commented Jan 12, 2017

OK. I've penciled this in for Saturday morning.

@finneyj

This comment has been minimized.

Show comment
Hide comment
@finneyj

finneyj Jan 12, 2017

Contributor

thanks!

Contributor

finneyj commented Jan 12, 2017

thanks!

@martinwork

This comment has been minimized.

Show comment
Hide comment
@martinwork

martinwork Jan 12, 2017

Contributor

Thanks for the heads up. Our app will be unaffected. I am working with an offline copy of the DAL and only updating semi-manually.

In our app's firmware, I have already made the UART code use notifications, not for this reason, but thinking it would enable improved throughput and buffering.

At first, when I read the above, I thought the TX/RX UUIDs were right already, so had a look round to check. I found that MicroBitUARTService's UUIDs are reversed compared to Nordic and mbed usage, but maybe not according to the documentation! Somehow, without realising this was different from the DAL, I have matched them to mbed's definition (see below) in our firmware and app.

This is what I found:

Nordic's description under the heading "UART/Serial Port Emulation over BLE" isn't clear to me. It seems to be reversed compared to actual usage.

At the end of the mbed UARTService header, there is a good explanation, saying that tx/rx are from the point of view of the external client - ie desktop/tablet/phone. It writes to rxCharacteristic=6e400003 and reads txCharacteristic=6e400002.

The app NordicSemiconductor/Android-nRF-UART writes to RX_CHAR_UUID=6e400002 and reads from TX_CHAR_UUID=6e400003. These UUIDs correspond to mbed's but the TX/RX names are reversed.

The MicroBitUARTService uses mbed's UUID #defines but writes to txCharacteristic=6e400002 and reads rxCharacteristic=6e400003, so the usage is reversed.

Contributor

martinwork commented Jan 12, 2017

Thanks for the heads up. Our app will be unaffected. I am working with an offline copy of the DAL and only updating semi-manually.

In our app's firmware, I have already made the UART code use notifications, not for this reason, but thinking it would enable improved throughput and buffering.

At first, when I read the above, I thought the TX/RX UUIDs were right already, so had a look round to check. I found that MicroBitUARTService's UUIDs are reversed compared to Nordic and mbed usage, but maybe not according to the documentation! Somehow, without realising this was different from the DAL, I have matched them to mbed's definition (see below) in our firmware and app.

This is what I found:

Nordic's description under the heading "UART/Serial Port Emulation over BLE" isn't clear to me. It seems to be reversed compared to actual usage.

At the end of the mbed UARTService header, there is a good explanation, saying that tx/rx are from the point of view of the external client - ie desktop/tablet/phone. It writes to rxCharacteristic=6e400003 and reads txCharacteristic=6e400002.

The app NordicSemiconductor/Android-nRF-UART writes to RX_CHAR_UUID=6e400002 and reads from TX_CHAR_UUID=6e400003. These UUIDs correspond to mbed's but the TX/RX names are reversed.

The MicroBitUARTService uses mbed's UUID #defines but writes to txCharacteristic=6e400002 and reads rxCharacteristic=6e400003, so the usage is reversed.

@drmpf

This comment has been minimized.

Show comment
Hide comment
@drmpf

drmpf Jan 12, 2017

While I am all for a BLE UART 'standard', the problem is there isn't one.

My app pfodApp (www.pfod.com.au) handles 7 different BLE Uart implemenations from various manufactures and this proposed change will not affect it as it already has that config for another 'nordic' style board, Adafruit SPI BLE boards. (The Arduino 101 'nordic' board is similar, but uses INDICATE)

However, the sheer volume of micro:bits in the wild seems, to me, to have defined already another standard.
If you change the micro:bit standard now, what happens to the existing micro:bits and those in the supply chain?
How do they get updated to the 'new' micro:bit UART standard?

I fear the result will be two micro:bit UART standards and a give rise to cases of that micro:bit app will not actually connect to that version of micro:bit.

Given the number of micro:bits already out there it would seem simpler to have Nordic change their (one) UART app to also accept the micro:bit UART standard.

drmpf commented Jan 12, 2017

While I am all for a BLE UART 'standard', the problem is there isn't one.

My app pfodApp (www.pfod.com.au) handles 7 different BLE Uart implemenations from various manufactures and this proposed change will not affect it as it already has that config for another 'nordic' style board, Adafruit SPI BLE boards. (The Arduino 101 'nordic' board is similar, but uses INDICATE)

However, the sheer volume of micro:bits in the wild seems, to me, to have defined already another standard.
If you change the micro:bit standard now, what happens to the existing micro:bits and those in the supply chain?
How do they get updated to the 'new' micro:bit UART standard?

I fear the result will be two micro:bit UART standards and a give rise to cases of that micro:bit app will not actually connect to that version of micro:bit.

Given the number of micro:bits already out there it would seem simpler to have Nordic change their (one) UART app to also accept the micro:bit UART standard.

@bluetooth-mdw

This comment has been minimized.

Show comment
Hide comment
@bluetooth-mdw

bluetooth-mdw Jan 13, 2017

Contributor

@martinwork yes, the root of the issue is that Nordic themselves have been inconsistent. They're going to address this on their side. We'll move to the spec I got from them yesterday.

Contributor

bluetooth-mdw commented Jan 13, 2017

@martinwork yes, the root of the issue is that Nordic themselves have been inconsistent. They're going to address this on their side. We'll move to the spec I got from them yesterday.

@bluetooth-mdw

This comment has been minimized.

Show comment
Hide comment
@bluetooth-mdw

bluetooth-mdw Jan 13, 2017

Contributor

@drmpf As you said, there's no standard for serial communications over Bluetooth low energy. The Nordic implementation is not a standard. All we're doing here is ensuring that we will be consistent with the specification for the Nordia UART service so that micro:bit works with the Nordic applications.

micro:bits don't have the UART service on them by default i.e. out of the box. It's a software module you have to explicitly include in your code. Anyone wanting or needing to update to the new version will simply need to recompile and flash the new hex file.

micro:bits themselves are not the issue really. The impact will be felt by client applications that use the UART service for exchanging arbitrary data. Such applications will need to be changed. It's unfortunate but that's the way it is. I myself will be impacted and will need to change the micro:bit Blue Android application. I intend to put clear user info in the application to ensure people know to recompile their micro:bit code, though I suspect 99.9% of people use micro:bit Blue with a hex file I provide anyway.

So summary: micro:bits in the wild are not affected unless they happen to have software on them which includes the UART service. We have no data on this but I feel very confident that it's a very small minority.

Contributor

bluetooth-mdw commented Jan 13, 2017

@drmpf As you said, there's no standard for serial communications over Bluetooth low energy. The Nordic implementation is not a standard. All we're doing here is ensuring that we will be consistent with the specification for the Nordia UART service so that micro:bit works with the Nordic applications.

micro:bits don't have the UART service on them by default i.e. out of the box. It's a software module you have to explicitly include in your code. Anyone wanting or needing to update to the new version will simply need to recompile and flash the new hex file.

micro:bits themselves are not the issue really. The impact will be felt by client applications that use the UART service for exchanging arbitrary data. Such applications will need to be changed. It's unfortunate but that's the way it is. I myself will be impacted and will need to change the micro:bit Blue Android application. I intend to put clear user info in the application to ensure people know to recompile their micro:bit code, though I suspect 99.9% of people use micro:bit Blue with a hex file I provide anyway.

So summary: micro:bits in the wild are not affected unless they happen to have software on them which includes the UART service. We have no data on this but I feel very confident that it's a very small minority.

@bluetooth-mdw

This comment has been minimized.

Show comment
Hide comment
@bluetooth-mdw

bluetooth-mdw Jan 13, 2017

Contributor

@drmpf fyi what I'm thinking of doing with micro:bit Blue is to examine the characteristics in the UART service. I'll require the right UUIDs of course but then look at the properties of each characterstic to deduce which of the two service versions I'm dealing with. Not pretty but eliminates the issue and given it's open source, is a useful example of dealing with a real world issue that happens in the... you know.... real world from time to time.

Contributor

bluetooth-mdw commented Jan 13, 2017

@drmpf fyi what I'm thinking of doing with micro:bit Blue is to examine the characteristics in the UART service. I'll require the right UUIDs of course but then look at the properties of each characterstic to deduce which of the two service versions I'm dealing with. Not pretty but eliminates the issue and given it's open source, is a useful example of dealing with a real world issue that happens in the... you know.... real world from time to time.

@drmpf

This comment has been minimized.

Show comment
Hide comment
@drmpf

drmpf Jan 13, 2017

Yes I already do that in pfodApp in addition to checking against 'known' Uart services.
However in pfodApp, I check every service UUID returned for a suitable set of characteristics that could be used for a UART and also then check the characteristics for Notify/Indicate settings.

Finally some manufacturers , just to be different, use just one characteristic UUID for both TX and Rx, so I have to allow for that 'standard' also.

What did Nordic say about registering their set of UART UUIDs as a BLE Standard?

drmpf commented Jan 13, 2017

Yes I already do that in pfodApp in addition to checking against 'known' Uart services.
However in pfodApp, I check every service UUID returned for a suitable set of characteristics that could be used for a UART and also then check the characteristics for Notify/Indicate settings.

Finally some manufacturers , just to be different, use just one characteristic UUID for both TX and Rx, so I have to allow for that 'standard' also.

What did Nordic say about registering their set of UART UUIDs as a BLE Standard?

@bluetooth-mdw

This comment has been minimized.

Show comment
Hide comment
@bluetooth-mdw

bluetooth-mdw Jan 14, 2017

Contributor

It's not a registration process. It's a lot more involved than that. But I believe Nordic are members of the SIG working group that has "serial" on its plate and so are doubtless discussing their service.

Contributor

bluetooth-mdw commented Jan 14, 2017

It's not a registration process. It's a lot more involved than that. But I believe Nordic are members of the SIG working group that has "serial" on its plate and so are doubtless discussing their service.

@bluetooth-mdw

This comment has been minimized.

Show comment
Hide comment
@bluetooth-mdw

bluetooth-mdw Jan 18, 2017

Contributor

I've been digging into this one and as usual there's more involved than meets the eye.

Using Notifications instead of Indications and switching the RX/TX characteristics around will make the service usable from the official Nordic nRF Toolbox application.

However......

Indications have the advantage of giving us a flow control mechanism. We know the client received the last message we sent it and so can now proceed to send the contents of the buffer again. This is how the implementation currently works.

If we do switch to using notifications, we either accept that we have no flow control and there therefore the mechanism is inherently unreliable, or we implement a form of flow control some other way.

The flow control issue has two aspects to it;

(1) Knowing the client received the message at the layer of ATT in the stack (if data doesn't make it to the device at all, we'll know from the link layer, even with notifications) and

(2) knowing the local Nordic framework on the micro:bit accepted and transmitted the notification message we gave it.

(1) can only really be addressed by using Indications. So that's not an option (if our goal in part is to stop using Indications!)

(2) This is largely about the availability of TX buffers in the Nordic stack and there being available connection events which will carry our packet(s). There needs to be feedback from the Nordic framework, via mbed about whether or not our notification request was successfully handled (because there was buffer space available). Or we need to query the state of the buffers before sending the notification. Or something like that.

My understanding from an issue I looked at last year where notifications were being "lost" (I may be out of date) is that the mbed APIs do not pass back errors from the Nordic APIs and so we will not know based purely on checking the result of an mbed API call, whether or not the Nordic framework transmitted our notification. If this is still the case, we either need to fix the mbed APIs (at least in this area) or conclude that the mbed APIs are not fit for purpose here and use the Nordic APIs directly.

Concrete suggestions from Nordic are:

Use sd_ble_tx_buffer_count_get() and wait until there are available buffers.

Keep track of the state ourselves by waiting for BLE_EVT_TX_COMPLETE before sending 
the next notification (or rather send a few notifications then wait for the same number 
of TX complete, in order to get more than one notification per connection event.)

Wait for the next BLE_EVT_TX_COMPLETE and retry whenever we get the 
BLE_ERROR_NO_TX_BUFFERS return value from sd_ble_gatts_hvx().

Either way, this is not a quick fix.

Contributor

bluetooth-mdw commented Jan 18, 2017

I've been digging into this one and as usual there's more involved than meets the eye.

Using Notifications instead of Indications and switching the RX/TX characteristics around will make the service usable from the official Nordic nRF Toolbox application.

However......

Indications have the advantage of giving us a flow control mechanism. We know the client received the last message we sent it and so can now proceed to send the contents of the buffer again. This is how the implementation currently works.

If we do switch to using notifications, we either accept that we have no flow control and there therefore the mechanism is inherently unreliable, or we implement a form of flow control some other way.

The flow control issue has two aspects to it;

(1) Knowing the client received the message at the layer of ATT in the stack (if data doesn't make it to the device at all, we'll know from the link layer, even with notifications) and

(2) knowing the local Nordic framework on the micro:bit accepted and transmitted the notification message we gave it.

(1) can only really be addressed by using Indications. So that's not an option (if our goal in part is to stop using Indications!)

(2) This is largely about the availability of TX buffers in the Nordic stack and there being available connection events which will carry our packet(s). There needs to be feedback from the Nordic framework, via mbed about whether or not our notification request was successfully handled (because there was buffer space available). Or we need to query the state of the buffers before sending the notification. Or something like that.

My understanding from an issue I looked at last year where notifications were being "lost" (I may be out of date) is that the mbed APIs do not pass back errors from the Nordic APIs and so we will not know based purely on checking the result of an mbed API call, whether or not the Nordic framework transmitted our notification. If this is still the case, we either need to fix the mbed APIs (at least in this area) or conclude that the mbed APIs are not fit for purpose here and use the Nordic APIs directly.

Concrete suggestions from Nordic are:

Use sd_ble_tx_buffer_count_get() and wait until there are available buffers.

Keep track of the state ourselves by waiting for BLE_EVT_TX_COMPLETE before sending 
the next notification (or rather send a few notifications then wait for the same number 
of TX complete, in order to get more than one notification per connection event.)

Wait for the next BLE_EVT_TX_COMPLETE and retry whenever we get the 
BLE_ERROR_NO_TX_BUFFERS return value from sd_ble_gatts_hvx().

Either way, this is not a quick fix.

@jamesadevine

This comment has been minimized.

Show comment
Hide comment
@jamesadevine

jamesadevine Jan 18, 2017

Contributor

@bluetooth-mdw What do Nordic use for flow control? What they suggested? It seems like flow control is pretty vital to a UART implementation. At the time of writing it, indications seemed the most natural way of obtaining flow control.

Why did Nordic not use indications? Could this be a case of developing the UART Service rather than simply adhering to the existing implementation?

Contributor

jamesadevine commented Jan 18, 2017

@bluetooth-mdw What do Nordic use for flow control? What they suggested? It seems like flow control is pretty vital to a UART implementation. At the time of writing it, indications seemed the most natural way of obtaining flow control.

Why did Nordic not use indications? Could this be a case of developing the UART Service rather than simply adhering to the existing implementation?

@bluetooth-mdw

This comment has been minimized.

Show comment
Hide comment
@bluetooth-mdw

bluetooth-mdw Jan 18, 2017

Contributor

They don't really have what I'd call a production implementation afaik. What they do have are sample implementations they ship with their SDKs. Afaik they do not implement any form of flow control.

I personally think the root of all this was calling it "UART" in the first place! What we have here is really a convenient, simple, "serial-like" capability for arbitrary data to be exchanged. "UART" is stretching things somewhat.

Yes, we could go our own way. The only downside is that it will not work with the nRF Toolbox application. We should definitely also change the UUIDs for the service and its characteristics if we do this. Arguably we'd only need to change the UUID of the characteristic which uses notifications but to avoid perpetuating more confusion, I'd say we should allocate UUIDs from the range I used for the custom services created specifically for micro:bit. If we go our own way that is.

I personally like the idea of it working with the Nordic application and would prefer we make that the primary consideration.

Contributor

bluetooth-mdw commented Jan 18, 2017

They don't really have what I'd call a production implementation afaik. What they do have are sample implementations they ship with their SDKs. Afaik they do not implement any form of flow control.

I personally think the root of all this was calling it "UART" in the first place! What we have here is really a convenient, simple, "serial-like" capability for arbitrary data to be exchanged. "UART" is stretching things somewhat.

Yes, we could go our own way. The only downside is that it will not work with the nRF Toolbox application. We should definitely also change the UUIDs for the service and its characteristics if we do this. Arguably we'd only need to change the UUID of the characteristic which uses notifications but to avoid perpetuating more confusion, I'd say we should allocate UUIDs from the range I used for the custom services created specifically for micro:bit. If we go our own way that is.

I personally like the idea of it working with the Nordic application and would prefer we make that the primary consideration.

@jamesadevine

This comment has been minimized.

Show comment
Hide comment
@jamesadevine

jamesadevine Jan 18, 2017

Contributor

@bluetooth-mdw Of course, I completely agree with regards to working with the Nordic implementation. I was suggesting was that we work with nordic to use indications as a form of flow control instead of no flow control at all, would there be any reason for them to object?

The other solutions they have suggested feel a bit "hacky" to me, we build on top of the BLE_API, we would be dodging that layer entirely.

Contributor

jamesadevine commented Jan 18, 2017

@bluetooth-mdw Of course, I completely agree with regards to working with the Nordic implementation. I was suggesting was that we work with nordic to use indications as a form of flow control instead of no flow control at all, would there be any reason for them to object?

The other solutions they have suggested feel a bit "hacky" to me, we build on top of the BLE_API, we would be dodging that layer entirely.

@finneyj

This comment has been minimized.

Show comment
Hide comment
@finneyj

finneyj Jan 18, 2017

Contributor

@bluetooth-mdw @jamesadevine

Collaborating with the folks at Nordic is the way to go IMHO, but if we have experiences and developments that improve the Nordic service, can we not look to inform the Nordic implementations and improve them?

e.g. why not work with the engineers at Nordic to enable a way to use indications for flow control for those apps that use it? Best of both worlds?

Contributor

finneyj commented Jan 18, 2017

@bluetooth-mdw @jamesadevine

Collaborating with the folks at Nordic is the way to go IMHO, but if we have experiences and developments that improve the Nordic service, can we not look to inform the Nordic implementations and improve them?

e.g. why not work with the engineers at Nordic to enable a way to use indications for flow control for those apps that use it? Best of both worlds?

@bluetooth-mdw

This comment has been minimized.

Show comment
Hide comment
@bluetooth-mdw

bluetooth-mdw Jan 18, 2017

Contributor

You're right. If we can persuade Nordic to change their app to support Indications as well as notifications (same Android API anyway), that solves that. We then just have to swap our UUIDs around.

Who's going to approach Nordic on this? I'm happy to as I'm already in conversation about this. Or is this a time for the Foundation in all it's might to step in?

Contributor

bluetooth-mdw commented Jan 18, 2017

You're right. If we can persuade Nordic to change their app to support Indications as well as notifications (same Android API anyway), that solves that. We then just have to swap our UUIDs around.

Who's going to approach Nordic on this? I'm happy to as I'm already in conversation about this. Or is this a time for the Foundation in all it's might to step in?

@PaulAustin

This comment has been minimized.

Show comment
Hide comment
@PaulAustin

PaulAustin Feb 9, 2017

I'll ditto the request to see the RX/TX match the direction used by the core MBED and nordic library. I've encountered a few other nRF51/nRF52 enabled devices and the console tools can't communicate with the MB-DAL based firmware. I prototyped a fix (https://github.com/PaulAustin/microbit-dal) that is working fine for me from tools like Adafruits Bluefruit LE connect app.

PaulAustin commented Feb 9, 2017

I'll ditto the request to see the RX/TX match the direction used by the core MBED and nordic library. I've encountered a few other nRF51/nRF52 enabled devices and the console tools can't communicate with the MB-DAL based firmware. I prototyped a fix (https://github.com/PaulAustin/microbit-dal) that is working fine for me from tools like Adafruits Bluefruit LE connect app.

@martinwork

This comment has been minimized.

Show comment
Hide comment
@martinwork

martinwork Feb 12, 2017

Contributor

With notify(), there is a sort of flow control possible using an onDataSent callback and checking the result of notify().

There is a danger in the fact that many BLE services use notify(), without checking the return value and without any coordination.

What flow control is there from central into microbit?

Contributor

martinwork commented Feb 12, 2017

With notify(), there is a sort of flow control possible using an onDataSent callback and checking the result of notify().

There is a danger in the fact that many BLE services use notify(), without checking the return value and without any coordination.

What flow control is there from central into microbit?

@gfwilliams

This comment has been minimized.

Show comment
Hide comment
@gfwilliams

gfwilliams Jul 18, 2018

Just to add that everyone really has agreed on the spec for Nordic UART, and it's the opposite of what micro:bit still does.

If you use the Adafruit Bluefruit devices/apps, Espruino, Nordic's app or cloud website, or Nordic's own libraries for Nordic UART central or peripheral they all use the flipped UUIDs and notifications.

I really think this should change - or at the very least you should change the UUIDs around to be something micro:bit specific so there is no confusion.

gfwilliams commented Jul 18, 2018

Just to add that everyone really has agreed on the spec for Nordic UART, and it's the opposite of what micro:bit still does.

If you use the Adafruit Bluefruit devices/apps, Espruino, Nordic's app or cloud website, or Nordic's own libraries for Nordic UART central or peripheral they all use the flipped UUIDs and notifications.

I really think this should change - or at the very least you should change the UUIDs around to be something micro:bit specific so there is no confusion.

@bluetooth-mdw

This comment has been minimized.

Show comment
Hide comment
@bluetooth-mdw

bluetooth-mdw Jul 19, 2018

Contributor

I agree we should bite the bullet and get this changed so that it is in line with other implementations and the popular tools work. I know the background to this as I discussed the problem with Nordic Support ages ago. Apparently one of their SDK releases has the characteristics the wrong way around... not sure about the indications vs notifications aspect of this. But consequently there are two different "definitions" of their UART service out in the wild and hence the confusion.

This is not a standard anyway. So let's just change it so it works.

I know I volunteered to do this ages ago but I don't have time these days. Should be a minor change to be honest. The important thing though is to summarise here what the proposed changes are and get agreement before proceeding.

Contributor

bluetooth-mdw commented Jul 19, 2018

I agree we should bite the bullet and get this changed so that it is in line with other implementations and the popular tools work. I know the background to this as I discussed the problem with Nordic Support ages ago. Apparently one of their SDK releases has the characteristics the wrong way around... not sure about the indications vs notifications aspect of this. But consequently there are two different "definitions" of their UART service out in the wild and hence the confusion.

This is not a standard anyway. So let's just change it so it works.

I know I volunteered to do this ages ago but I don't have time these days. Should be a minor change to be honest. The important thing though is to summarise here what the proposed changes are and get agreement before proceeding.

@finneyj

This comment has been minimized.

Show comment
Hide comment
@finneyj

finneyj Jul 20, 2018

Contributor

@gfwilliams @bluetooth-mdw

Thanks both for raising the profile of this issue again and providing the background.

If a de facto standard has emerged, it makes sense to align to it IMHO.

The challenge is of course supporting any micro:bit apps that are already out there through such a change to keep things as smooth as possible.

@martinwork @bluetooth-mdw Thoughts on what we can do to make an easy transition for you? As an ideal, I guess we'd like to ensure a "nothing breaks" model... and we will have old versions of apps and kids micro:bit programs out there to deal with.

Contributor

finneyj commented Jul 20, 2018

@gfwilliams @bluetooth-mdw

Thanks both for raising the profile of this issue again and providing the background.

If a de facto standard has emerged, it makes sense to align to it IMHO.

The challenge is of course supporting any micro:bit apps that are already out there through such a change to keep things as smooth as possible.

@martinwork @bluetooth-mdw Thoughts on what we can do to make an easy transition for you? As an ideal, I guess we'd like to ensure a "nothing breaks" model... and we will have old versions of apps and kids micro:bit programs out there to deal with.

@gfwilliams

This comment has been minimized.

Show comment
Hide comment
@gfwilliams

gfwilliams Jul 20, 2018

Just a thought - but could you make both UUIDs writable, with notifications/indications on each? I think it may then work with both old and new apps.

gfwilliams commented Jul 20, 2018

Just a thought - but could you make both UUIDs writable, with notifications/indications on each? I think it may then work with both old and new apps.

@finneyj

This comment has been minimized.

Show comment
Hide comment
@finneyj

finneyj Jul 20, 2018

Contributor

@gfwilliams You mean like a virtual wireless auto-MDIX?!?

I think I want one!

I guess there are situations where the connecting apps don't transmit first though. We could define polarity based on an notification registration, but what to do if the app doesn't register for a notification and doesn't transmit? Default to the de facto standard I guess?

Contributor

finneyj commented Jul 20, 2018

@gfwilliams You mean like a virtual wireless auto-MDIX?!?

I think I want one!

I guess there are situations where the connecting apps don't transmit first though. We could define polarity based on an notification registration, but what to do if the app doesn't register for a notification and doesn't transmit? Default to the de facto standard I guess?

@gfwilliams

This comment has been minimized.

Show comment
Hide comment
@gfwilliams

gfwilliams Jul 23, 2018

I could be wrong, but I don't think you have to figure out which way around to do it - you could just use both at the same time...

  • When sending from the micro:bit, Indicate on 0x0002 and Notify on 0x0003
  • When either characteristic is written to, treat that as data coming in to the micro:bit

Do you think that's likely to work @bluetooth-mdw?

gfwilliams commented Jul 23, 2018

I could be wrong, but I don't think you have to figure out which way around to do it - you could just use both at the same time...

  • When sending from the micro:bit, Indicate on 0x0002 and Notify on 0x0003
  • When either characteristic is written to, treat that as data coming in to the micro:bit

Do you think that's likely to work @bluetooth-mdw?

@bluetooth-mdw

This comment has been minimized.

Show comment
Hide comment
@bluetooth-mdw

bluetooth-mdw Jul 24, 2018

Contributor

Personally I would go with the "break it model". Rather than make things more complicated and introduce more potential confusion, just make the change and communicate it via social media. I bet very few people use it.

Contributor

bluetooth-mdw commented Jul 24, 2018

Personally I would go with the "break it model". Rather than make things more complicated and introduce more potential confusion, just make the change and communicate it via social media. I bet very few people use it.

@martinwork

This comment has been minimized.

Show comment
Hide comment
@martinwork

martinwork Jul 24, 2018

Contributor

I'm for keeping it simple too. I think dropping indications would mean it doesn't need to keep a live copy of the currently-called-txCharacteristic, releasing a little RAM.

Contributor

martinwork commented Jul 24, 2018

I'm for keeping it simple too. I think dropping indications would mean it doesn't need to keep a live copy of the currently-called-txCharacteristic, releasing a little RAM.

@finneyj

This comment has been minimized.

Show comment
Hide comment
@finneyj

finneyj Jul 24, 2018

Contributor

All good points. It is important to note that we are just talking about a subset of the BLE functionality here,with the other services remain unchanged.

Still, if we're going cold turkey, we should at least align a updated release of the affected micro:bit apps we know about with this change.

@martinwork @bluetooth-mdw Do your apps make use of this BLE uart service?

Contributor

finneyj commented Jul 24, 2018

All good points. It is important to note that we are just talking about a subset of the BLE functionality here,with the other services remain unchanged.

Still, if we're going cold turkey, we should at least align a updated release of the affected micro:bit apps we know about with this change.

@martinwork @bluetooth-mdw Do your apps make use of this BLE uart service?

@finneyj

This comment has been minimized.

Show comment
Hide comment
@finneyj

finneyj Jul 24, 2018

Contributor

Also tagging @jaustin in here, in case he's aware of other apps that we may want to reach out to. Is the Scratch team using this maybe? others?

Contributor

finneyj commented Jul 24, 2018

Also tagging @jaustin in here, in case he's aware of other apps that we may want to reach out to. Is the Scratch team using this maybe? others?

@smartyw

This comment has been minimized.

Show comment
Hide comment
@smartyw

smartyw Jul 24, 2018

Contributor

One of my apps uses it (@bluetooth-mdw here in disguise!)

Contributor

smartyw commented Jul 24, 2018

One of my apps uses it (@bluetooth-mdw here in disguise!)

@martinwork

This comment has been minimized.

Show comment
Hide comment
@martinwork

martinwork Jul 24, 2018

Contributor

@finneyj No. Our apps are using a custom implementation.

Contributor

martinwork commented Jul 24, 2018

@finneyj No. Our apps are using a custom implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment