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

Drive TX line high before switching to input #13098

Closed
wants to merge 2 commits into from

Conversation

SteveCEvans
Copy link
Member

This PR attempts to fix the issues tackled by #13095.

The proposed solution mentioned at #13095 (comment) of only disabling the TX output once at 100ms have passed since a received character, resulted in the original issue resurfacing which was the FC dropping into DFU mode when a connected WS VTX was powered off.

The PR drives the TX line high with a pull-up before switching to an input with a pull-up. For #13095 to have any positive effect it must be either preventing the TX being sampled low, or the TX line is actually being pulled down by the attached device. This PR addresses the former.

@dxs94 could you please test this PR and report back.

@github-actions
Copy link

Do you want to test this code? You can flash it directly from Betaflight Configurator:

  • Simply put #13098 (this pull request number) in the Select commit field of the Configurator firmware flasher tab (you need to Enable expert mode, Show release candidates and Development).

WARNING: It may be unstable. Use only for testing!

@haslinghuis haslinghuis added this to the 4.5 milestone Sep 28, 2023
@blckmn
Copy link
Member

blckmn commented Sep 28, 2023

AUTOMERGE: (FAIL)

  • github identifies PR as mergeable -> FAIL
  • assigned to a milestone -> PASS
  • cooling off period lapsed -> PASS
  • commit count less or equal to three -> PASS
  • Don't merge label NOT found -> FAIL
  • at least one RN: label found -> PASS
  • Tested label found -> FAIL
  • assigned to an approver -> PASS
  • approver count at least three -> FAIL

@SteveCEvans
Copy link
Member Author

The unit tests are working for me running under Linux.

CC version: clang version 10.0.0-4ubuntu1  Target: x86_64-pc-linux-gnu Thread model: posix InstalledDir: /usr/bin
CXX version: clang version 10.0.0-4ubuntu1  Target: x86_64-pc-linux-gnu Thread model: posix InstalledDir: /usr/bin

Whereas the cloud build is:

CC version: Ubuntu clang version 14.0.0-1ubuntu1.1 Target: x86_64-pc-linux-gnu Thread model: posix InstalledDir: /usr/bin
CXX version: Ubuntu clang version 14.0.0-1ubuntu1.1 Target: x86_64-pc-linux-gnu Thread model: posix InstalledDir: ```

@SteveCEvans SteveCEvans force-pushed the tx_drive_high branch 4 times, most recently from 81c040a to 8c14c97 Compare September 28, 2023 22:37
@SteveCEvans
Copy link
Member Author

@haslinghuis As you'll see I've made a number of commits to fix some, but not all of the unit tests. Something is broken around the definition of STATIC_UNIT_TESTED I suspect. Has the unit test script been changed to undefine UNIT_TEST - see build_config.h.

@haslinghuis
Copy link
Member

haslinghuis commented Sep 29, 2023

Clang versions has been updated in the workflow image: actions/runner-images#8329

EDIT: should be fixed using #13099

@dxs94
Copy link

dxs94 commented Sep 29, 2023

Tested with BF configurator 10.10.0RC1; selected BF 4.5.0.Z.
Commit #13098 is NOT working (no stick movement - no crossfire telemetry).
Same conditions, commit #13095 shows stick inputs.

@SteveCEvans
Copy link
Member Author

Unit test changes backed out as this is not the place to fix that issue.

@SteveCEvans
Copy link
Member Author

@dxs94 Thanks for testing. I've just pushed a commit which adds a new debug mode, UART_TX.

Could you please log with that. I presume this is still on an F4?

This will log the following:

Debug Value
debug[0] Count of times the UART 2 TX line's state is read
debug[1] Count of times the UART 2 TX line is found to be pulled low
debug[2] Count of times the UART 2 TX line's is switched to an input

If we see debug[1] incrementing with everything powered on... that's bad.

@dxs94
Copy link

dxs94 commented Sep 29, 2023

Hi yes still the same F411 board from GEPRC P16 as in #12902. Unfortunately no black box on this one so no logging :(

@SteveCEvans
Copy link
Member Author

In light of testing there are evidently issues with some receivers so closing this and moving to #13100.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: COMPLETED
Development

Successfully merging this pull request may close these issues.

None yet

4 participants