Disabling permanent UART and Bluetooth interaction confusing, causes lockout with Float package

[lukash]

Setting the option App Settings -> General -> Enable Permanent UART to false can lead to some very confusing behavior making the user spend a lot of time/effort to figure out. There's a handful of subtle interactions at play here:

• When it's set to false (assuming BLE module connected to it), the link is actually still enabled after boot (due to app_uartcomm_start() in main() enabling it unconditionally right after the app config was set directly above it). It's only disabled on the first App config write. Not sure this is intentional (to prevent user from locking themselves out via BT and needing to access the USB port), but seems to actually be a bug because:
• The help says "You may want to disable this to prevent access to your VESC over BLE.". Due to the above point it doesn't really do that.
• When the user has the Float package installed, it sets a config value (Mahony KP) in the app conf right at the start, meaning with the Float package the option actually will lock one out of the app over BT.

What happened to me is that when flashing FW and Float package, I picked a wrong app conf backup and having the Float package installed, I locked myself out permanently. But (not realizing bad configuration of the option was at play), I spent hours debugging the BT connection sometimes working, sometimes working only until config write and sometimes not working at all.

What saved me from even more hours debugging this was mentioning it to Victor from customwheel.shop, who knew the solution right away. He said this has bitten him quite hard in the past as well.

Less important but related, when the option is false, it means the UART link to the BLE module is down, but the module is still live, discoverable and upon connection it says "failed to read FW version". Obviously YMMV but maybe VESC is reaching the level of "polish" where there should be an option to disable the BLE module completely, so that it doesn't show up when you scan.

[vedderb]

Nice find, sounds like a tricky issue. I haven't used ble-modules that connect over UART for a while now.

Here is an attempt to make it behave as expected: https://github.com/vedderb/bldc/commit/f9da202eb8707f12415e0f50371a76475a68aa45

Regarding making the ble-module not show up when disabled, that is probably best done in the NRF-firmware. Could also consider updating the VESC Tool error message to something like: Failed to read FW version. If you are using UART, make sure that the port is enabled, connected correctly and uses the correct baudrate.

[vedderb]

Message updated: https://github.com/vedderb/vesc_tool/commit/bcfcc49bfce018c37d5d232fc367b638402d4d6f

[lukash]

Hmm... not sure how big of a fan I am of the message :)

The main issue with this error message I think is it represents a wide range of errors that may occur on the path from the app to the VESC, including BLE communication issues, BLE module issues, UART issues and a handful of misconfigurations. Fixing this (having separate error messages) is nontrivial, but would be very nice (I'm sure it's one of the bigger category of problems new and sometimes old users face).

That aside, the message now contains a lot of technical details, that still aren't entirely exhaustive and don't always apply. E.g. I'd guess most people nowadays use built-in modules (integrated into the VESC unit and connected to internal UART). For those, the rx/tx connection is irrelevant and I'm not sure it's possible to set the baud rate. OTOH the "Enable Permanent UART" option isn't mentioned, and for a new user it still may not be obvious that option is relevant.

I don't think the message should be made any longer. Getting back to this being the message the user sees on most failed connection attempts, and some of them not being technically inclined, the message should be succinct, nice to look at and not contain potentially irrelevant details that will frustrate these users because they don't understand them. Instead, I think a good solution would be to have a short message with a link to a knowledge base article on troubleshooting connection issues. Then the message should go like: "Failed to establish connection. For details on troubleshooting connection issues, visit LINK".

[vedderb]

Setting up a separate knowledge base and pointing to it is 1000x more work for me than just updating the message. So either you can have an updated message now or a knowledge base at some undefined time in the future.

Also, I don't agree with having a separate place where people can go to debug the problem. So far my approach has been to avoid questions where possible in the first place and when that is not possible have the information right there in VESC Tool. That is why every parameter has a help button and why the wizards and page groups have information where you use them. The VESC package description is also viewable from VESC Tool and none of this requires an internet connection.

This error message does not have that many possible causes and all of them can be listed on a phone screen even without a scroll bar. I see no reason to not just spell everything out in the message. If it is unclear now more details can be added to the message than what I added now.

[lukash]

No problem. I just gave you a PoV of a user that also happens to develop professional products with polished user interfaces.

I understand both your arguments for having everything in one place as well as the amount of work setting up a knowledge base would be.

An advantage of a knowledge base would also be that it can be improved by the community and that content can be added whenever needed, not just when a new tool version is released.

Not saying the message isn't an improvement. I probably didn't make it clear enough I'm fine with the message being the immediate solution due to practical constraints... And I don't even need the message for myself anymore, now that I've spent hours figuring it out from the source code. I just wanted to help make the user experience a bit better for others.

Not sure the "Enable Permanent UART" option should also be mentioned in the message then?