Open matthieuvigne opened 4 months ago
Hi @matthieuvigne, I am looking at your PR and in general the idea of changing the baudrate. There are multiple issues that need to be taken into consideration:
Without any changes on the code I was able to make a file download work up to a baud rate of 921600
Hi @andreagilardoni
Thanks for your feedback. The way I see it, the first two points you mention would be resolved if we use a fix baudrate. There is however the point about reliability which I hadn't thought about - this could be a reason why one would want a lower baudrate in a noisy environment. I haven't done any formal test but succeeded in transmitting data packets up to 2000000 - but these were all short tests, I could benchmark that more in depth if needed. However I think this may be less important if we keep the baudrate user-customizable: we can simply give a recommendation, but leave it up to the user to try a higher speed and reduce it if he sees issues - whereas if we set a fix baud rate we need to be certain of the success rate.
For an auto-detect, I was thinking of a hacky solution to try all "standard" baudrates one by one from the renesas until the expected reply is received. This is not ideal however: it doesn't work if the user uses a custom baudrate ; sending data at the 'wrong' baudrate for the esp32 could, if we are unlucky, lead to erroneous readings by the esp32 that would hinder detection. Do you think this is relevant enough ? Or should we focus on getting a fix higher baudrate to work ?
Hi @matthieuvigne, I have made some considerations about this topic. We need to have the right compromise between reliability and speed, for this board we want to focus on reliability. I think adding an api for baudrate change will introduce a lot of issues that need to be addressed that will impact reliability of the board. For example, even if we change the baudrate statically on this repo, we need to add a baudrate autodetection algorithm (that needs to be reliable and fast) to account for esp32-renesas code version mismatch.
In my opinion this will add a lot of complexity on the code that will introduce more problems than solutions.
I think the best solution is that you change the baudrate for your use case, matching your requirements. You don't have to account for the issues we may face changing this value. After all the idea behind this repo is to be simple, customizable with an interface that a maker can change, otherwise we would have used SPI instead of UART with a different exchange data format.
One thing that will be more than appreciated is to provide measures of SNR wrt baudrates, so that if someone wants to change the baudrate can look at this and understand what is the best possible value he can use.
I would also suggest you to try this PR https://github.com/arduino/ArduinoCore-renesas/pull/349 which addresses some issues of the AT parser when dealing with timeout responses, that may happen when dealing with higher speeds.
In the meanwhile, thanks for your contribution, we can discuss your ideas and understand if they can be merged.
the WiFiS3 baud rate change function, for example WiFi.setUartBaudRate(br), would not be executed automatically. the user would invoke it in their sketch. setUartBaudRate would send the AT command to change the baud rate. if the command fails (with old fw), then the baud rate stays the default. If the command returns ok, thensetUartBaudRate changes the baud rate on the Uno side. on reset the baudrate has to return to default. in the main mcu and in the fw.
@JAndrassy If there is an error on the renesas core and it is restarted, without the esp restarting, you need to perform baudrate detection in order to reset it. For the time being, I think, the best possible solution is to have it changed on both sides for whoever needs it, so there is no need to implement the autodetection of baudrate.
if the esp32 resets it returns to the default baud rate of 115 200
in order to reset it from fw you need to send an AT command, otherwise you need to unplug the device
Thanks for the details @andreagilardoni - I understand your decision not to merge, this clearly has more consequences than I initially thought. I'll try the PR you propose, and maybe do some SNR measurements if I find the time.
I am trying to make an autodetect for the baudrate in my spare time, i'll update you if I succeed
Related to https://github.com/arduino/uno-r4-wifi-usb-bridge/issues/55
This implements the "AT+UART" command to change the baudrate on the fly, keeping the default at 115200.
While this works in practice, it has side effects due to the fact that, when RESET is pressed on the board, the ESP32 is not reset - thus, it keeps the previously configured speed. This means that if you upload code to change the baudrate, then upload another code that uses a different baudrate, the second code won't work until you reboot the whole board.
This is not user-friendly, but I don't see a way around it (apart from testing all common baudrate until getting a valid reply, which isn't really practical). The good thing about this code though is that it's compatible with the current Arduino version: for a standard user who doesn't touch the baudrate, everything still works. Increasing the baudrate would be 'advanced usage' where we assume that the user is aware and ok with this side effect.
However, I would personally be more in favor of changing this baudrate once and for all for a new hard-coded value. Indeed I think that this reboot side effect can be quite annoying, or misleading, to the point where it may prevent its usage (no one will want to package this in their code if they know it creates such side effect) ; keeping the feature as a form of 'low-level advanced usage' means it will likely seldom or never be used. The only significant downside of this is that it breaks backward compatibility with version 1.2.0 of https://github.com/arduino/ArduinoCore-renesas. I think this can be handled in a satisfactory way by making a new release of ArduinoCore-renesas that's compatible with both versions (assuming 0.4.2 uses the fastest speed, then we could have ArduinoCore-renesas first try this fast speed, then if it fails revert back to 115200).