I am using r2351 , file hci.c , line 871 has a problem
if (hci_stack->substate > 1 || COMMAND_COMPLETE_EVENT(packet, hci_reset))
Notice that a OR is used, shouldn't that be an AND? Isn't there a risk of the next command being sent before the unit has fully reset?
I am seeing this problem right now, monitoring my UART using an logic analyzer. The command complete does not arrive before the subsequent command
Comment #1
Posted on Feb 5, 2014 by Swift Dogok the problem is very complex, it's not a battle between OR or AND, it's the substate number that's the problem. The problem is that if a new baud rate does not need to be adjusted, the substate skips to 4 immediately, which skips waiting for the reset to complete
To fix this, a change needs to be added to hci_run's substate-machine's switch-case
I have attached a tested patch
- hci.c.issue377.patch 1.97KB
Comment #2
Posted on Feb 5, 2014 by Swift Doghmm... this presents a problem, the patch I submitted only solves the problem if the user does not wish to change the baud rate.
According to BCCMD documentation, the command complete reply to the change baud rate command will be returned at the new baud rate. But the BTstack code does not advance to changing the host baud rate until a command complete is received.
Also, it is highly likely that the UART transmitter is FIFO'ed and there's a good chance that the baud rate can be changed in the middle of a packet if the coder isn't careful enough about waiting for the FIFO to be drained first. (On embedded, this is easy to detect, I'm not sure about other platforms.)
This means that there is a VERY small timing window when the host baud rate can be changed, immediately after the last byte in the FIFO is sent completely, but before the controller can reply.
For BlueCore chips, a simple fix is to simply move the set_baud function right after the hci_send_cmd_packet. However, I have no idea how TI brand chips handles a baud change request.
Comment #3
Posted on Feb 6, 2014 by Swift Oxbaud rate change: at least for the modules I've worked with, the module will send the command compete with the old baud rate before changing to the new one. Otherwise, it would be impossible for the module to report an unsupported baud rate (as the host would already switch to the unsupported baud rate). In practice, I don't see a reason to change the baud rate after the initial startup, and there, the Bluetooth module isn't supported to send anything.
I'll look at the "wait for reset complete" later, but that clearly makes sense. (that HCI startup is more or less the oldest code in BTstack and I don't really like the ugly substate management with the extra cases).
Comment #4
Posted on Feb 6, 2014 by Swift DogFor BlueCore, one way is to edit the UART setting in the persistent storage, but I want to avoid that. The second way is through BCCMD.
BCCMD command reference clearly states "This BCCMD command immediately configures the UART’s baud rate and settings. The GETRESP for this command will probably return to the host at the new UART settings."
Although your state machine will not be able to handle this because it is not a "command complete", I'll have to hack around that.
My RN42HCI starts up at 115200 baud by default, I'd like to keep it that way, and only make temporary baud rate changes, not permanent.
The developer should be responsible for making sure that the baud rate is supported.
Comment #5
Posted on Feb 6, 2014 by Swift OxCould you send me the docs for that (I've seen them before but don't have them with me). While I'd argue that changing it after the command complete would make sense, we need a solution if CSR does change it before that.
Also.. "|command will probably return to the host at the new UART settings" is a bit unclear at least. Does it or does it not use the new setting. :) Did you see it on the logic analzyser?
So, if CSR indeed needs that, I'd suggest to add a configuration flag to the hci_uart_config_t that specifies if the UART speed is changed before or after the command complete.
In general, that's clear flaw of the Bluetooth specification to not define this. It wouldn't have been so hard to do so.
Comment #6
Posted on Jul 30, 2014 by Swift Ox(changing topic as wait for reset to complete has been rewritten and works with CSR8811)
Status: Accepted
Labels:
Type-Defect
Priority-Medium