Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Serial Refactor] 2.7RC1 does not power TX16S AUX uarts for all protocols #1766

Closed
stronnag opened this issue Mar 30, 2022 · 18 comments · Fixed by #1848
Closed

[Serial Refactor] 2.7RC1 does not power TX16S AUX uarts for all protocols #1766

stronnag opened this issue Mar 30, 2022 · 18 comments · Fixed by #1848
Milestone

Comments

@stronnag
Copy link
Contributor

On a TX16S, the AUX UARTS are not powered for all protocols

Protocol Port Power
Telem Mirror Yes
LUA Yes
GPS Yes
Telem In Yes
CLI No
Debug No

Most recently tested with 5ffe757 2022-03-30

The AUX UARTS should be powered for all protocols (e.g. to use bluetooth dongle).

@pfeerick
Copy link
Member

pfeerick commented Mar 30, 2022

I believe there will be a bluetooth option in the future also (thus basically removing the bluetooth compile option also as it will always be available). For debug I don't think power should be on, as it's intended for comms only, and for CLI I think it should be in the domain of CLI to turn the power on and off as desired (but I suspect that is not an option yet?).

@stronnag
Copy link
Contributor Author

This has nothing to do with BT in the firmware, it's using an external BT dongle on a AUX port --- transparent serial.
So if I want to use a BT dongle for debug or CLI when I don't have a VCP cable available, you're telling me I can't.

@raphaelcoeffic
Copy link
Member

raphaelcoeffic commented Mar 30, 2022

@stronnag please check here:

edgetx/radio/src/serial.cpp

Lines 192 to 250 in 777b0a4

static void serialSetupPort(int mode, etx_serial_init& params, bool& power_required)
{
switch (mode) {
#if defined(DEBUG) || defined(CLI)
case UART_MODE_DEBUG:
case UART_MODE_CLI:
params.baudrate = DEBUG_BAUDRATE;
break;
#endif
#if !defined(BOOT)
case UART_MODE_TELEMETRY_MIRROR:
// TODO: query telemetry baudrate / add setting for module
power_required = true;
#if defined(CROSSFIRE)
if (modelTelemetryProtocol() == PROTOCOL_TELEMETRY_CROSSFIRE) {
params.baudrate = CROSSFIRE_TELEM_MIRROR_BAUDRATE;
break;
}
#endif
params.baudrate = FRSKY_TELEM_MIRROR_BAUDRATE;
break;
case UART_MODE_TELEMETRY:
if (modelTelemetryProtocol() == PROTOCOL_TELEMETRY_FRSKY_D_SECONDARY) {
params.baudrate = FRSKY_D_BAUDRATE;
params.rx_enable = true;
power_required = true;
}
break;
case UART_MODE_SBUS_TRAINER:
params.baudrate = SBUS_BAUDRATE;
params.word_length = ETX_WordLength_9;
params.parity = ETX_Parity_Even;
params.stop_bits = ETX_StopBits_Two;
params.rx_enable = true;
power_required = true;
break;
#if defined(LUA)
case UART_MODE_LUA:
params.baudrate = LUA_DEFAULT_BAUDRATE;
params.rx_enable = true;
power_required = true;
break;
#endif
#if defined(INTERNAL_GPS)
case UART_MODE_GPS:
params.baudrate = GPS_USART_BAUDRATE;
params.rx_enable = true;
power_required = true;
break;
#endif
#endif
}
}

And pay special attention to these lines:

power_required = true;

So the short answer is: it is by design, but it's not written in stone ;-)

@stronnag
Copy link
Contributor Author

OK, so it is a feature that CLI and DEBUG are not available for such devices.

@pfeerick
Copy link
Member

pfeerick commented Mar 30, 2022

It's more that that is an unexpected use case. As Raphael said though, this is not set in stone. There is no reason an UI option couldn't be added to allow for the power to be toggled on or off in those modes if there is enough interest for such an addition.

@rotorman
Copy link
Member

rotorman commented Mar 30, 2022

I can relate to the original post - if the serial port is not OFF, power should IMO be turned on, also for DEBUG or CLI.
An option in UI for the user to select this, would be even neater indeed.

@rotorman rotorman reopened this Mar 30, 2022
@raphaelcoeffic
Copy link
Member

@rotorman feel free to change it ;-)

@rotorman
Copy link
Member

@stronnag would you care to test #1848

grafik

@stronnag
Copy link
Contributor Author

@stronnag would you care to test #1848

Sure, I'll get back to you soonish. Thanks.

@stronnag
Copy link
Contributor Author

On my TX16S

  • AUX ports are powered by command in the UI for all protocols
  • Don't seem to be able to set a working CLI on AUX1/2; other tested protocols (LUA, TelemMirror are OK)
  • CLI works on VCP

@rotorman
Copy link
Member

rotorman commented Apr 19, 2022

Thank you for testing!

On my TX16S

* AUX ports are powered by command in the UI for all protocols

Meaning you can enable and disable power OK, as per checkbox?

* Don't seem to be able to set a working CLI on AUX1/2; other tested protocols (LUA, TelemMirror are OK)

This is likely unrelated to #1848
For TX16S need to use 400.000 baud 8N1, other radios use mostly 115.200 baud 8N1. Did you try with 400.000 baud?

@stronnag
Copy link
Contributor Author

Thank you for testing!

On my TX16S

* AUX ports are powered by command in the UI for all protocols

Meaning you can enable and disable power OK, as per checkbox?

Yes

  • Don't seem to be able to set a working CLI on AUX1/2; other tested protocols (LUA, TelemMirror are OK)

This is likely unrelated to #1848 For TX16S need to use 400.000 baud 8N1, other radios use mostly 115.200 baud 8N1. Did you try with 400.000 baud?

No, my BT dongle is locked to 115200. I suspected that would be the problem. Let me try a USB-TTL adapter.

@stronnag
Copy link
Contributor Author

Took a while to find an USB-TTL dongle that allows non-traditional baud rates (i.e. accepting 400000 rather than deciding that 460800 is close enough). No difference though, still no CLI on AUX; so for a different issue (at least if it's the same on MacOS and Windows).

Not withstanding your efforts, perhaps we should just close this again and accept that CLI is not a option for AUX (and the UI should enforce this limitation to avoid user frustration)?

@rotorman
Copy link
Member

rotorman commented Apr 19, 2022

@stronnag Just tested #1848 with TX16S mkII, AUX1 set to CLI, SparkFun DEV-09717 hooked up to AUX1, hTerm connected with 400.000 baud 8N1. Works beautifully for me.
Are you sending CR+LF at the end? Type help accompanied by CR+LF? This is the output I get:

> help
beep [<frequency>] [<duration>]
ls <directory>
read <filename>
readsd <start sector> <sectors count> <read buffer size (sectors)>
testsd 
play <filename>
reboot [wdt]
set <what> <value>
serialpassthrough <port type> <port number>
help [<command>]
gps <baudrate>|$<command>|trace
>

@stronnag
Copy link
Contributor Author

I have the CR+LF.
Works fine over the VCP, USB-TTL device works fine at 400000 with loopback. TX16 Mk1, Arch Linux (5.17), firmware compiled with gcc 11.20. I'll try an older compiler tomorrow.
However, this PR is fine, the power is working, before we too far off topic.

@pfeerick pfeerick linked a pull request Apr 20, 2022 that will close this issue
@pfeerick
Copy link
Member

Or take the compiler out of the equation - try the github build of the PR? I find going back to / sticking with the GH builds when testing removes environmental variables if there are potential issues. 😀

https://github.com/EdgeTX/edgetx/suites/6172150719/artifacts/216510037
or
https://buddy.edgetx.org/#/dev/flash/pr?version=pr-1848%405682286173f09de936ac0640cb4f90a48a70a66e

@pfeerick pfeerick added this to the 2.8 milestone Apr 20, 2022
@stronnag
Copy link
Contributor Author

stronnag commented Apr 20, 2022

Thanks @pfeerick, the first option worked OK (CLI /AUX1 / 400000 baud). Interesting, inav firmware has some serial issues with 11.20 as well.
For my own satisfaction, I'll have to give 10.20 a go.

@stronnag
Copy link
Contributor Author

and for the record, 10.20 works as well.
Thanks both.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants