Major refactor #3
Conversation
I'm not going to try and break this into smaller commits - sorry. Switch USB audio code to C++. Add new HA and HAManager classes. Add PseudoAtomic library for proper variable syncing. This allows compiling in Release mode. The shared audio buffer is now a C++ class. Various other refactors.
The string literal (to smaller char array) is not legal in C++
|
Finally got my pico. What console settings are you using to connect to the compiled-in serial port? I've tried both minicom and kermit with various settings and haven't seen any output. |
|
Settings are in the readme, but here they are:
|
|
The other problem you might have is if you connect too late, you might not see any new output. That's why I prefer UART debugging myself, because my debug probe serial is always connected. |
|
Oh, just checked, without hearing aids connected, there is no output. Maybe I need to add some scanning logs back in... EDIT: There is some output, but it might be too late by the time the USB serial reconnects. |
The previous method did not account for BTStack reusing the data buffer
| uint8_t type; | ||
| uint8_t event_type; | ||
| uint8_t address_type; | ||
| uint8_t rssi; |
There was a problem hiding this comment.
rssi is actually an int8_t. The values I have observed usually sit from -100 to about -30 or so.
|
I'm still having trouble during the service discovery part. I think you are trying to initiate the service discovery before the connection has been secured, so the hearing device ignores the request. I've attached an hci dump: pico_output.log |
| Complete, | ||
| }; | ||
|
|
||
| constexpr uint16_t buff_size_sdu = 161; |
There was a problem hiding this comment.
This needs to be at least 167, to account for the l2cap header
There was a problem hiding this comment.
As far as I'm aware, the SDU is the application data size, and does not contain l2cap headers. That's the PDU, and I believe btstack handles that automatically.
|
Thanks for that log @thewierdnut It shows me that your device doesn't support LE Secure connections, so pairing fails.
Yeah, I think I know what I'm doing wrong there. I've tried changing some stuff this morning, but it hasn't worked yet and I've got to go to work. I'll pick back up this evening. Sigh... I'm back to the pairing game... |
|
I've made a few changes, I will be interested to see if that helps for your device @thewierdnut I'm just setting the default connection parameters up front, and doing away with the parameter update. I'm trying to perform the ASP notification subscription after the L2CAP connection is created (this is what Android appears to do, at least according to my android HCI snoop). I also now attempt to delete a pairing on reencryption failure. |
|
I'll give it a try. A small note about the android setup state machine... As soon as the Gatt is connected, it triggers a call to UpdateBleConnParams, and if it ever gets a parameter update with the wrong parameters, it just calls UpdateBleConnParams again. It will actually connect to the gap socket as soon as the the encryption is complete and the characteristics are known (discovered or cached), which means the connection parameter update process and the service discovery can actually run in parallel. Once the connection parameters are correct and the socket is available, then it finally calls the ACP start. |
|
Still isn't happy |
|
Why oh why oh why? What am I doing wrong? |
|
I get the distinct idea that we are somehow overcomplicating this. I'm tempted to steal a btstack sample app and just statically connect to the psm via a mac address, just to see if it is stable. Also... what do you want to accomplish with this particular peer review? Do you want me to review code style? design? functionality? I think we have moved past that somewhat into troubleshooting a specific hearing aid. |
Yeah, maybe give that a go. Or feel free to hack away at my code.
Just whatever springs to mind really. This troubleshooting is fine, because it's a major refactor in draft state, so I have no worries making major changes. If you can suggest better ways of doing things, feel free, as this is my first ever microcontroller and bluetooth project, so I'm still learning stuff. |
Upon re-connecting a HA, there's no need to rediscover it's services and characteristics.
|
While I'm not caching the advertising packets, I decided to implement a cache of successfully connected HA's so that subsequent reconnections don't have to rediscover services and characteristics. Anecdotally, the connection seems less stable after that change, but that could be anything, because bluetooth... I think once we can get your Starkey's working, I will like to focus on stream stability. I think I will probably need to modify btstack a little to get access to the available outgoing credits. |
|
Just did a quick bit of logging, it seems the audio streaming never falls behind the PCM stream, at least not to a significant degree. I tried logging with a difference of 8 and 4 packets, and the only time I saw any logging was when things had already gone wrong. I seem to have some state issues... Maybe I partly just need to find some efficiencies as well. I keep having to remind myself this is running on a dual core ARM Cortex M0+ with 256K of RAM. The M0+ is basically ARM's cheapest and slowest 32-bit CPU. I may also have to experiment with |
|
Another option might be to investigate the SDK alarm API to schedule the streaming stuff instead of using an infinite loop. |
|
Here is the latest log output (with advertisements disabled) that you requested via slack: |
|
Thanks, so it seems it's fine up until attempting to create the l2cap channel, at which point it has a melt-down, and for the cherry on top, it refuses to pair after reconnecting. The l2cap error is #ifdef ENABLE_L2CAP_LE_CREDIT_BASED_FLOW_CONTROL_MODE
static void l2cap_handle_hci_le_disconnect_event(l2cap_channel_t * channel){
if (l2cap_send_open_failed_on_hci_disconnect(channel)){
l2cap_cbm_emit_channel_opened(channel, L2CAP_CONNECTION_BASEBAND_DISCONNECT);
} else {
l2cap_emit_channel_closed(channel);
}
l2cap_free_channel_entry(channel);
}
#endifThe error is based on what static int l2cap_send_open_failed_on_hci_disconnect(l2cap_channel_t * channel){
// open cannot fail for for incoming connections
if (channel->state_var & L2CAP_CHANNEL_STATE_VAR_INCOMING) return 0;
// check state
switch (channel->state){
case L2CAP_STATE_WILL_SEND_CREATE_CONNECTION:
case L2CAP_STATE_WAIT_CONNECTION_COMPLETE:
case L2CAP_STATE_WAIT_REMOTE_SUPPORTED_FEATURES:
case L2CAP_STATE_WAIT_OUTGOING_SECURITY_LEVEL_UPDATE:
case L2CAP_STATE_WAIT_CLIENT_ACCEPT_OR_REJECT:
case L2CAP_STATE_WAIT_OUTGOING_EXTENDED_FEATURES:
case L2CAP_STATE_WAIT_CONNECT_RSP:
case L2CAP_STATE_CONFIG:
case L2CAP_STATE_WILL_SEND_CONNECTION_REQUEST:
case L2CAP_STATE_WILL_SEND_LE_CONNECTION_REQUEST:
case L2CAP_STATE_WAIT_LE_CONNECTION_RESPONSE:
case L2CAP_STATE_EMIT_OPEN_FAILED_AND_DISCARD:
return 1;
case L2CAP_STATE_OPEN:
case L2CAP_STATE_CLOSED:
case L2CAP_STATE_WAIT_INCOMING_EXTENDED_FEATURES:
case L2CAP_STATE_WAIT_DISCONNECT:
case L2CAP_STATE_WILL_SEND_CONNECTION_RESPONSE_INSUFFICIENT_SECURITY:
case L2CAP_STATE_WILL_SEND_CONNECTION_RESPONSE_DECLINE:
case L2CAP_STATE_WILL_SEND_CONNECTION_RESPONSE_ACCEPT:
case L2CAP_STATE_WILL_SEND_DISCONNECT_REQUEST:
case L2CAP_STATE_WILL_SEND_DISCONNECT_RESPONSE:
case L2CAP_STATE_WILL_SEND_LE_CONNECTION_RESPONSE_DECLINE:
case L2CAP_STATE_WILL_SEND_LE_CONNECTION_RESPONSE_ACCEPT:
case L2CAP_STATE_INVALID:
case L2CAP_STATE_WAIT_INCOMING_SECURITY_LEVEL_UPDATE:
return 0;
default:
// get a "warning" about new states
btstack_assert(false);
return 0;
}
}In other words, it could be any of the first 12 l2cap states. Very helpful btstack... |
|
If you look at the raw hci traffic, you see this just prior to the logged disconnect: Basically, it quit because the hearing aid terminated the connection on purpose. My devices tend to just cut the bluetooth connection when they see something they don't like. |
The timestamp is in ms from boot
|
@thewierdnut I've added a basic timestamp to logs, that should give more context, without majorly screwing up timings like a HCI dump does. |
|
Apologies for the lack of updates. I started trying some stuff, and then my plans were all derailed early this week. |
|
No worries. Life happens. I haven't had time to work on it either. |
Note, btstack has no API for this, so send raw HCI command
|
New Pico SDK is out. I think I'm just gonna merge this as-is, then start hopefully making stability improvements. |
I wasn't too happy with how... organic the code had gotten, so I decided to do a major refactor.
Quoting myself from the commit message:
No real improvements on the audio side. I am currently doing zero buffering, so the connection is unstable, but properly synced audio. Some ideas might be to detect underruns and insert silent packets to compensate, or figure out how to properly buffer without the stereo going out of sync.
I'd be interested to see your thoughts on this refactor @thewierdnut . I think it's better than what I had before, but I'm biased.