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

Invert serial rx #3139

Merged
merged 8 commits into from
Dec 13, 2015
Merged

Invert serial rx #3139

merged 8 commits into from
Dec 13, 2015

Conversation

mikeller
Copy link
Contributor

Hi there.

As discussed earlier today, I have added the option for the 9XR pro to receive FrSky D telemetry that is inverted on pin 5 of the module connector, and feed it through its internal inverter to get it to the right format.

I have made this option only available for the 9XR pro. This might be too restrictive, as other sky9x based boards (or potentially the Taranis) might have a switchable on-board inverter too, but I have no way to verify this.

hope this helps, cheers
Michael Keller

@@ -52,7 +52,7 @@ DELAY = 5
# PCB version
# Values: 9X, 9X128, 9X2561, 9XR, 9XR128, 9XR2561, GRUVIN9X, MEGA2560, SKY9X, 9XRPRO, AR9X, TARANIS
# Old values STD and STD128 are mapped to 9X and 9X128 for compatibility
PCB = 9X
PCB = 9XRPRO
Copy link
Member

Choose a reason for hiding this comment

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

Your personal makefile changes shouldn't be committed! Hint: use command line instead for your builds, preferably in a script for convenience

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, that was a misunderstanding on my part of how github does pull requests. I assumed the pull request would only cover the changes in the branch, but not in master. So I added my build config to master, and all the changes for the feature in my branch. If you look at the changes by changeset, the first two changesets in the pull request are not meant to be in there, all the changes for the feature are in the last changeset.

Copy link
Member

Choose a reason for hiding this comment

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

It would have worked that way if you had created your branch before commtting your makefile changes on master (or alternatively rebased it to a point of master before these changes were committed)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On 11/12/15 18:46, Andre Bernet wrote:

It would have worked that way if you had created your branch /before/
commtting your makefile changes on master (or alternatively rebased it
to a point of master before these changes were committed after the fact)

Definitely. That was not what I wanted though, I wanted to make the
changes to the makefile permanent in my own branch, so that I could
build with my settings.

Having the makefile under revision control is a bit problematic anyway,
or at least it would be nice for people building from source to have a
config file where they could store their build settings that did not
interfere with version control.

(I have not done much with git in the past, so I will first have to get
a grip on git-ish features like rebase.)

GCS$/CC/E/IT d- s+ a C++ UL+++/S++ P L++ E-
W++ N o? K? w O(++) M-- V+ PS+ PE+ Y? PGP+ t
5? X R tv b++ DI++ D++ G e+++ h---- r+++ y+++

Copy link
Member

Choose a reason for hiding this comment

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

It will obviously always be a mess to keep mods in the makefile. Whch is again why it's not a good idea to do it... it also limits you to a single config.

I instead use multiple scripts for the various configs I use regularly in the form of

make clean
make PCB=TARANIS PCBREV=REVPLUS HELI=YES GVARS=YES TRANSLATIONS=EN DEBUG=NO LUA=YES MIXERS_MONITOR=YES TIMERS=3 USB=JOYSTICK CLI=NO DANGEROUS_MODULE_FUNCTIONS=NO TARANIS_INTERNAL_PPM=NO

dfu-util-static -a 0 -d 0483:df11 --dfuse-address 0x08000000 -D opentx.bin

for example for the Taranis, that cleans, builds the version I want, then flashes it automatically.

@@ -283,7 +286,14 @@ void telemetryWakeup()
{
#if defined(CPUARM)
uint8_t requiredTelemetryProtocol = MODEL_TELEMETRY_PROTOCOL();
#if defined(REVX)
uint8_t requiredSerialInversion = g_model.moduleData[EXTERNAL_MODULE].invertedSerial;
if (telemetryProtocol != requiredTelemetryProtocol
Copy link
Member

Choose a reason for hiding this comment

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

On the same line, or it's more difficult to read!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Argh, again my work habits showing through. (We have a rule that no line may be longer than screen width.)
Will try to improve my context switching.

bsongis added a commit that referenced this pull request Dec 13, 2015
@bsongis bsongis merged commit a8c794f into opentx:master Dec 13, 2015
@bsongis
Copy link
Member

bsongis commented Dec 13, 2015

It's all good for me, just the little cosmetics issue I saw, perhaps you may fix it directly in 'master'? Thanks for this good job!

@mikeller
Copy link
Contributor Author

You're welcome. Happy to see that I can add some value to this project.

One more question, just to be absolutely clear: The development process is that now these changes will have to be manually grafted into next? I am happy to do the work, I just want to make sure I am not messing doing work that cannot be used, or messing up a merge down the line.

@bsongis
Copy link
Member

bsongis commented Dec 13, 2015

Yes, manually, the 2 branches have diverged a lot now

@projectkk2glider
Copy link
Member

This pull is for #3138

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.

None yet

4 participants