Open ghost opened 7 years ago
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?
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.
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.
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.
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...
OK. I've penciled this in for Saturday morning.
thanks!
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.
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.
@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.
@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.
@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.
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?
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.
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.
@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?
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.
@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.
@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?
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?
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.
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?
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.
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.
@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.
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 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?
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...
Do you think that's likely to work @bluetooth-mdw?
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.
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.
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?
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?
One of my apps uses it (@bluetooth-mdw here in disguise!)
@finneyj No. Our apps are using a custom implementation.
Hi, is this issue still active? I'd like to use Nordic UART in MakeCode.
So far, I forked microbit & microbit-dal and modify codes as PaulAustin demonstrated. It works fine with mbed. I compiled my sample code with "yotta install tadakado/microbit". I can connect microbit UART from Nordic UART compatible app, such as Bluefruit iPhone app. (Still I can't see UART in the service list, but it does have the service)
I prefer to use Nordic UART in MakeCode. Because it's easy to communicate with other Nordic BLE device and integrate with various MakeCode extensions. Especially it's quite useful for kids. My current project is to send iPhone's ANCS (call/email/sns...) message to a microbit-car through an ANCS-UART repeater. I made the ANCS-UART repeater and my kids will make the microbit-car part. When phone calls, microbit will dance!
Another ping on this issue. I appreciate that changing to TX - Notify and RX - Write is tricky as an ecosystem has developed around the current implementation. However, as discussed further up the thread, it seems like micro:bit needs to do a breaking change one way or another. Either changing UUIDs or changing to what Nordic have as their current implementation. I would prefer to go with fitting in with the NUS "standard" because there are other resources out there for BLE-Central implementations of it that could be leveraged.
I had to revisit the tx/rx for microbit V2 codal libraries. Swapping the advertised IDs is. Looking at putting in a pull request but what to still check out a few bits. For those that venture in this thread.
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.