improve bit timing for 19200 bl protocol

[tridge]

this fixes the bit timing to be more robust

The first issue was that serialWriteChar() was only 9 bits long, whereas a byte is 10 bits (including stop bit). It worked with 9 bits as a pullup on the other end (on the flight controller) finished off the byte, but that is susceptible to noise as a pullup isn't as strong as a GPIO write. That meant that functions like send_BAD_ACK() which only send one byte relied on the pullup to get the byte completely out. Note that it worked for sendString() as that added an extra bit time for the stop bit.

The next problem is that serialreadChar() didn't reset the timer, so it's timeout handling wasn't consistent depending on where it was called from (when the last timer reset happened). serialreadChar() also didn't check the mid-point of the start bit and didn't check the stop bit at all. This made it more likely it would see noise as a valid byte.

serialreadChar() also should return bool so that when there is a bad start bit or stop bit the command can be abandoned. The CRC usually caught the error, but we may as well use the extra bits to make it more robust.

finally debugging stats support is added to help debug the bit timing

[freasy]

Just looked at the code, lgtm.

[AlkaMotors]

There are no problems that this will introduce that i can see but some things to take note of in the existing code.

There is a stop bit between bytes except for single byte transfers. Even for single byte transfers the line is written high and then the esc pull-up is enabled on that pin and set to read immediately.. It;s a hard transition to high. I can not remember why exactly i did this but i do know that it was intentional to only have the push_pull stop bit on transfers larger than 1 byte and the pull-up stop byte on single byte transfers. Having the stop bit in writeChar was actually moved in the past to the sendString function and i made some confusing note about it here.
https://github.com/AlkaMotors/AM32_Bootloader_F051/blob/d2a455bbaab6eff7597c2a53317a080778cc1c6e/Core/Src/main.c#L468

At this moment i can't see any reason not to change it back but we might encounter the same situation that caused me to do this originally.

The timer is not really inconsistent. It's set to timeout if the line is low for 200ms after the last transmit or read. Timer 2 is reset in the delayMicrosecond function.

readChar does check the stop bit.. The line must be continually high for 250 microseconds after the 8 data bits.. then the packet gets processed.

Checking the start bit mid bit time is a good plan.

[tridge]

Even for single byte transfers the line is written high and then the esc pull-up is enabled on that pin and set to read immediately.. It;s a hard transition to high.

the pattern is like this:

    setTransmit();
    serialwriteChar(0x30);             // good ack!
    setReceive();

but setReceive() does this:

    gpio_mode_set_input(input_pin, GPIO_PULL_NONE);

which means in the current master code it leaves the pin floating for the duration of the stop bit, so I think for single byte replies we're actually relying on a pullup on the flight controller? Or have I missed something else?

Ahh! I think I see the issue. In the F051 bootloader you linked setReceive() uses LL_GPIO_PULL_UP:
https://github.com/AlkaMotors/AM32_Bootloader_F051/blob/main/Core/Src/main.c#L699
but I based the common bootloader on the F421 bootloader, which does this:

void setReceive(){
input_port->cfgr = (((((input_port->cfgr))) & (~(((input_pin * input_pin) * (0x3UL << (0U)))))) | (((input_pin * input_pin) * GPIO_MODE_INPUT)));
received = 0;
}

and that does not set the pull field on the input port, so pullup/down is set to whatever it was previously.
and the last set that I can see in the F421 code is this one after checkForSignal():

	gpio_mode_set( GPIO_MODE_INPUT, GPIO_PULL_NONE, input_pin);

https://github.com/AlkaMotors/AT32F421_AM32_Bootloader/blob/initial_upload/Src/main.c#L662
note that there is a commented out PULL_UP in setTransmit():
https://github.com/AlkaMotors/AT32F421_AM32_Bootloader/blob/initial_upload/Src/main.c#L223

so I think we really do have a bug and this PR fixes it. The bug is not in the F051 bootloader, but is in the F421 bootloader, and then came into the common bootloader as I used F421 as the basis (as that was the only ESC I had when I started!)

[AlkaMotors]

Yes, ah they are different! Well good catch, the changes look fine to me.

[tridge]

Yes, ah they are different! Well good catch, the changes look fine to me.

i've changed setReceive() to set pullup, which I think is the right thing to do, it just wasn't what the F421 bootloader did, but as discussed above I think that was just a bug