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

AP_VideoTX: Tramp: Add half-duplex to port config #28806

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

menschel
Copy link
Contributor

@menschel menschel commented Dec 4, 2024

This Patch fixes a minor annoyance with Tramp.
The protocol is half-duplex, yet the corresponding bit needs to be set manually in options.
This has already been done for Smart Audio.

BTW: Documentation does not even tell about this bit.
https://ardupilot.org/plane/docs/common-vtx.html#immersionrc-tramp

Copy link
Collaborator

@andyp1per andyp1per left a comment

Choose a reason for hiding this comment

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

Thanks! Can I ask that you set the correct baud rate as well? Pretty sure its fixed for tramp

@menschel
Copy link
Contributor Author

menschel commented Dec 5, 2024

You did that already, Andy.

https://github.com/ArduPilot/ardupilot/pull/21005/files#diff-fb699e6f83e01febd7de07fbf9e7d45899e384afff32823ce259be6f0380c597R25

But I had a bracket issue none the less. In my setup, pull-down on tx and rx are not necessary. Should I add them none the less to keep consistency with SmartAudio?

i.e.

port->set_options((port->get_options() & ~AP_HAL::UARTDriver::OPTION_RXINV)
            | AP_HAL::UARTDriver::OPTION_HDPLEX | AP_HAL::UARTDriver::OPTION_PULLDOWN_TX | AP_HAL::UARTDriver::OPTION_PULLDOWN_RX);

I have tested this and it makes no difference at all on my setup. I'll set pull-down for consistency then.

@menschel menschel force-pushed the pr-fix-tramp-uart-options branch 2 times, most recently from f0fd0ce to b9b5033 Compare December 5, 2024 11:52
Tramp is half-duplex, so this should be set
automatically like already done for SmartAudio.

Also set pull-down options for consistency with
SmartAudio.
@menschel menschel force-pushed the pr-fix-tramp-uart-options branch from b9b5033 to c98707e Compare December 5, 2024 13:44
@menschel
Copy link
Contributor Author

menschel commented Dec 6, 2024

-2

I have some InternalError 0x10000000 with this patch, have to reverse engineer where this comes from.

params_restored = (1U << 28), //0x10000000 268435456

Seems to be some fighting between tramp trying to set options and parameter storage.

I don't understand it yet, will keep digging.
https://github.com/ArduPilot/ardupilot/blob/master/libraries/AP_Param/AP_Param.cpp#L381

@andyp1per
Copy link
Collaborator

-2

I have some InternalError 0x10000000 with this patch, have to reverse engineer where this comes from.

params_restored = (1U << 28), //0x10000000 268435456

Seems to be some fighting between tramp trying to set options and parameter storage.

I don't understand it yet, will keep digging. https://github.com/ArduPilot/ardupilot/blob/master/libraries/AP_Param/AP_Param.cpp#L381

That indicates some kind of hardware failue or crash. Maybe there is some npe in your code.

@menschel
Copy link
Contributor Author

menschel commented Dec 6, 2024

It may have something to do with me changing between this SW, #28810 and AP 4.5.7 .

I have to get another tramp vtx.

I'm currently violating engineers code by working on the system that is actually flying instead of the technical mock-up.

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 this pull request may close these issues.

2 participants