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

User controllable serial port power #1848

Merged
merged 14 commits into from
Apr 24, 2022
Merged

User controllable serial port power #1848

merged 14 commits into from
Apr 24, 2022

Conversation

rotorman
Copy link
Member

@rotorman rotorman commented Apr 18, 2022

Solves #1766 by adding checkboxes into UI, for the user to explicitly enable or disable the power output on supply pins next to serial ports available on some radios (presently only TX16S has this feature).

Two example screenshots from physical TX16S mkII:

grafik

grafik

As USB VCP has no controllable power, no check box is presented for it.

The augmented serial port section in radio.yml, corresponding to the last screenshot:

serialPort: 
   AUX1:
      mode: SBUS_TRAINER
      power: 0
   AUX2:
      mode: GPS
      power: 1
   VCP:
      mode: CLI
      power: 0

Internally, the g_eeGeneral.serialPort was expanded with this PR to 8-bits per serial port, lower 4 bits are used for mode (0 = OFF, 1 = Telemetry Mirror, 2= Telemetry In, 3 = SBUS Trainer, 4 = LUA, 5 = CLI, 6 = GPS), 5th bit 8th bit for power state (0 = OFF, 1 = ON), the bits 5 to 7 remain presently unused.
Update: the power state bit was in a later commit changed to the most-significant bit due to suggestion by @pfeerick

Thx to @raphaelcoeffic for numerous tips with YAML.

Needs Companion support - @elecpower yelp!

Should we limit this feature presently to TX16S target only?

Thx to @raphaelcoeffic for numerous tips with YAML.
Tested with TX16S mkII.
@pfeerick
Copy link
Member

Is there any reason for the power bit to not be the highest, so that it isn't in the middle of the array if more options are added later? Not that it really makes any difference either way.

I think the question should more be "what other transmitters support soft power control of the serial power pins... if the answer is none... the answer is pretty clear ;)

@pfeerick pfeerick added this to the 2.8 milestone Apr 19, 2022
@pfeerick pfeerick added the enhancement ✨ New feature or request label Apr 19, 2022
@rotorman
Copy link
Member Author

rotorman commented Apr 19, 2022

Is there any reason for the power bit to not be the highest, so that it isn't in the middle of the array if more options are added later? Not that it really makes any difference either way.

I can change this to the MSBit if that is desired?

@rotorman rotorman changed the title User controllable serial port power User controllable serial port power for TX16S Apr 19, 2022
@rotorman
Copy link
Member Author

I think the question should more be "what other transmitters support soft power control of the serial power pins... if the answer is none... the answer is pretty clear ;)

I am presently aware of TX16S only, and thus limited the GUI additions in this PR to TX16S.

@rotorman
Copy link
Member Author

rotorman commented Apr 19, 2022

Is there any reason for the power bit to not be the highest, so that it isn't in the middle of the array if more options are added later? Not that it really makes any difference either way.

Changed in the latest commit the power bit to the most significant bit in the configuration byte, as suggested.

@elecpower
Copy link
Collaborator

Needs Companion support - @elecpower yelp!

@rotorman hear you

radio/src/serial.cpp Outdated Show resolved Hide resolved
@pfeerick
Copy link
Member

pfeerick commented Apr 20, 2022

I am presently aware of TX16S only, and thus limited the GUI additions in this PR to TX16S.

No, better that, you limited it via a feature flag ... thus it should just be a matter of adding that flag to any other transmitters with soft serial power control ;)

Changing the position of the power bit was really just up for discussion, but thank you :) Just made more sense to me organisation wise, until we need more bits! :-O

@elecpower
Copy link
Collaborator

Still testing but so far I have found a problem with the radio sim in that it is not reading the power value correctly ie always unchecked. Not wishing to point fingers but maybe it is related to the yml to new bit position.

Screenshot from 2022-04-20 23-59-40

serialPort:
AUX1:
mode: SBUS_TRAINER
power: 0
AUX2:
mode: GPS
power: 1
VCP:
mode: CLI
power: 0

@rotorman
Copy link
Member Author

@elecpower Thank you not only for Companion additions, but also pointing me to the right direction - you were perfectly correct about the issue stemming from power bit position shift - I needed to add 3 YAML_PADDING bits to accommodate the change as well. The PR should be in working condition again.

@elecpower
Copy link
Collaborator

Confirm power value now interpreted correctly.

However, there is a bug in the simulator which I'm not sure will show up in the real radio. Steps to replicate in Ubuntu 20.04:

  1. Companion new file and new model
  2. Radio Settings Hardware
  3. Set AUX1 to SBUS Trainer and power checked
  4. Simulate radio
  5. Radio SYS hardware and values match
  6. Close sim
  7. Companion change AUX1 from SBUS Trainer to OFF and power to unchecked
  8. Simulate radio
  9. Radio SYS hardware and port has old values

Would appear the memory space is not initialised so could show anything rather than old values as in my example.

@rotorman I've finished my testing so I'll hand back over to you to continue

@elecpower
Copy link
Collaborator

I noticed during testing that the serial port mode dropdown choices can vary between Companion and the simulator if the libsim has not been compiled with CLI and/or Debug.
Is this an issue, and if, so should Companion radio profile have a couple of Build Options check boxes?

@raphaelcoeffic
Copy link
Member

I noticed during testing that the serial port mode dropdown choices can vary between Companion and the simulator if the libsim has not been compiled with CLI and/or Debug. Is this an issue, and if, so should Companion radio profile have a couple of Build Options check boxes?

No, we should assume defaults as used by the GH builds.

@pfeerick
Copy link
Member

Or rather, perhaps this means libsim needs to be compiled with the same options as the radio, if they are missing? ;)

@pfeerick pfeerick self-assigned this Apr 23, 2022
@pfeerick
Copy link
Member

pfeerick commented Apr 23, 2022

Preliminary testing suggests there is discrepancy between how Companion and firmware are working... i.e. on the radio, you can toggle the port power regardless of the mode (which IMO is valid as one is not technically dependant on the other, but this also means you you can use the port for power for something else ) but it seems that Companion is only writing the power value when the port has a mode set (i.e. breaking that capability).

On a side note, I also notice Companion is not setting the USB-VCP port on TX16S to CLI by default (which is still ok, as the radio will populate this itself if it is missing). This does however mean when setting up a blank profile, CLI is available for AUX1, AUX2, when really it should already be reserved by VCP (btw, can you update that to USB-VCP please while you're in the neighbourhood :) )

@rotorman
Copy link
Member Author

I am in favor of being able to turn on or off AUX[1|2] power, irrespective of the mode selected, and even if the mode is OFF (e.g. to just tap 5V for some DIY mod w/o data lines involved).
@elecpower would you be so kind to match this behaviour in Companion plz as well?

@pfeerick
Copy link
Member

pfeerick commented Apr 23, 2022

Radio and companion testing now done... UI is working fine, companion is nuking power state as I expected it would if mode is "OFF".

An inconsistency in the AUX1 power state - it seems to be ON by default at powerup (in contrast to the UI state - checkbox was OFF), and you have to toggle it on and off for it to actually turn off. AUX2 is fine - it is off if set off, and toggles normally. This is also reflected in the bootloader - i.e. AUX1 is on in the bootloader, and AUX2 is off.

@rotorman
Copy link
Member Author

rotorman commented Apr 23, 2022

Thank you for testing! Hmm... I wonder if it stems from uninitialized g_eeGeneral.serialPort being evaluated during boot/start, as there is no difference in firmware code how AUX1 and AUX2 power are being handled. Need to investigate with ICD.

@rotorman
Copy link
Member Author

rotorman commented Apr 23, 2022

I had a look at bootloader code and the serial driver is loaded and initialized ONLY, if built with DEBUG:

#if defined(DEBUG)
initSerialPorts();
#endif

and
if(DEBUG)
set(BOOTLOADER_SRC
${BOOTLOADER_SRC}
../../../../../${STM32LIB_DIR}/STM32F4xx_StdPeriph_Driver/src/stm32f4xx_usart.c
../../../../../serial.cpp
../../../../../targets/common/arm/stm32/aux_serial_driver.cpp
../../../../../targets/common/arm/stm32/stm32_usart_driver.cpp
)
endif()

Hooked up a scope to TX16S AUX power pins and noticed that with 2.7.0-release AUX1 PWR gets ramped up in bootloader as well, whereas AUX2 remains down. Thus, at least the behaviour in bootloader seems to be unrelated to this PR.
Nevertheless, we could change it. In the commit below, I added code to explicitly power down the serial port power supply pins in bootloader:

@rotorman
Copy link
Member Author

rotorman commented Apr 23, 2022

Found my bug, now the serial power state should come up correctly, irrespective of the mode selected on boot-up (also when serial mode is OFF).

When bootloader is running, the serial power delivery pins are turned off (except the little glitch on AUX1 during entering bootloader, that happens before the bootloader main() runs).

I tested all 4 selection possibilities on TX16S (power for AUX1, AUX2 both off, only AUX1 power on and so on) with a scope. Re-booted also after making a selection to make sure the power is delivered as was selected after restart as well - all seems OK to me now.

@elecpower
Copy link
Collaborator

Radio and companion testing now done... UI is working fine, companion is nuking power state as I expected it would if mode is "OFF".

Changed Companion to always save serial port settings

Copy link
Member

@pfeerick pfeerick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed, power states are behaving exactly as they should for AUX1 and AUX2, on radio. Momentary bootloader 'glitch' of AUX1 power state is perfectly fine.

I've just pushed a fix for "Telemetry In" not beeing interpreted correctly, but everything else has tested ok, so once a companion test passes it's in.

image
image

TELEMETRY_MIRROR
SBUS_TRAINER
LUA
CLI
GPS
DEBUG

were all good.

@pfeerick pfeerick merged commit f5b1b53 into main Apr 24, 2022
@pfeerick pfeerick deleted the SerPWR branch April 24, 2022 01:53
@pfeerick pfeerick changed the title User controllable serial port power for TX16S User controllable serial port power Sep 26, 2022
@calderra99
Copy link

sorry for jumping in.

i wonder if there's a way that we can assign the function for the UART power availability to a toggle switch? that way we can use itu for an ON-OFF switch for a device attached or drawing power from the said UART, say maybe like an audio Bluetooth module or a telemetry Bluetooth module

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ✨ New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Serial Refactor] 2.7RC1 does not power TX16S AUX uarts for all protocols
5 participants