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

Add support for multi-protocol trainer #6896

Merged
merged 7 commits into from Oct 22, 2019
Merged

Conversation

raphaelcoeffic
Copy link
Member

Allows for using the new Rx protocols in multi-protocol module for trainer input. 16 channels supported.

@robustini
Copy link
Contributor

robustini commented Oct 5, 2019

Thanks @raphaelcoeffic for this!
I would like to test it, I have compiled the firmware with your commits but I can't find the option to enable it on my X10S Express.
The Multimodule is already in RX mode.
Only T16 and X9D can do this?

EDIT: I didn't have debug enabled, now I see it in X10.
Pay attention that in the T16 I think there is some mistake when choosing the type of trainer, strange things appear after Master/Multi section.
I think this happens because in my T16 I have installed the bluetooth module, so having declared it happens a bit of a mess.
Imho you should consider this combination

20191005_111352
20191005_111400

@robustini
Copy link
Contributor

robustini commented Oct 5, 2019

In the trainer channel monitor it works without any problem, but only if i disable the X10S internal module (IRSM), and disable it what would it do?
Since the system uses the telemetry channel must it be disabled on the internal module?
This would be quite limiting.
Given that normally in a trainer situation the channels that are used are almost always only 4, at most 5 or 6, wouldn't it be possible to make a sort of sharing able to maintain the telemetry, lowering the number of trainer channels?
8 in the vast majority of cases they would be too many.

@raphaelcoeffic
Copy link
Member Author

I think this happens because in my T16 I have installed the bluetooth module, so having declared it happens a bit of a mess. Imho you should consider this combination

Sorry but this is still too hacky for me now. We might consider this later, but for now, the defs related to trainer are already messy enough.

Since the system uses the telemetry channel must it be disabled on the internal module? This would be quite limiting.

This requires indeed an internal module that does not use the S.PORT line (such as ISRM, or internal multi). In your case, the issue has been fixed by @kilrah.

Sharing the S.PORT between the internal module for telemetry and the external for trainer signal would probably not work. S.PORT uses a master/slave bus. Inserting random packages their would most probably conflict with the master.

@raphaelcoeffic raphaelcoeffic added this to the OpenTX 2.3.2 milestone Oct 5, 2019
@robustini
Copy link
Contributor

robustini commented Oct 5, 2019

This requires indeed an internal module that does not use the S.PORT line (such as ISRM, or internal multi). In your case, the issue has been fixed by @kilrah.

Hi Raphael, i've ISRM on X10S Express (D16 in this model) but keep telling me I need to disable the internal rf, I've already tried the @kilrah fix, same issue here.
As soon as I disable it the trainer works.
I don't rule out something wrong myself, but I don't think so.

20191006_000439

@kilrah
Copy link
Member

kilrah commented Oct 5, 2019

Your capture shows you are not running a build with my fix.

Besides that there is still an issue, won't work if both are enabled at power up. But if you disable the ISRM then enable it again trainer continues to work.

@robustini
Copy link
Contributor

@kilrah compiled from this branch... i try again!

https://github.com/opentx/opentx/commits/raphaelcoeffic/multi-trainer

@robustini
Copy link
Contributor

robustini commented Oct 5, 2019

If this reflects the correct compilation to the right commit (i think) now it doesn't work even if I disable the internal module, the multimodule version appears but the trainer doesn't work.
Waiting for better times... :P

20191006_010435

@raphaelcoeffic
Copy link
Member Author

@robustini sorry, I had a typo in the last version that prevented the feature to work at all...

@robustini
Copy link
Contributor

robustini commented Oct 6, 2019

@robustini sorry, I had a typo in the last version that prevented the feature to work at all...

Thanks @raphaelcoeffic, with the typo fix works again, but only with the internal module disabled, and now it doesn't ask me to disable the internal rf but it tells me to enable the MULTI_TELEMETRY in the external module... @kilrah?
Here is the problem documented in a video.

https://youtu.be/NrcTnr1WZgw

@robustini
Copy link
Contributor

robustini commented Oct 6, 2019

@raphaelcoeffic As you can see in my video above the trainer limits now overwhelm 100%, they arrive here at 102.4%, previously to your fix at least here they seemed ok (+100/-100).

ppmInput[ch] = (value - 1024) * 500 / 820;

Try to replace 820 with 810.

@raphaelcoeffic
Copy link
Member Author

they arrive here at 102.4%

This is necessary so that you get 100% in the mixers. In fact, the trainer screen should be corrected instead.

@robustini
Copy link
Contributor

they arrive here at 102.4%

This is necessary so that you get 100% in the mixers. In fact, the trainer screen should be corrected instead.

Nice! However it continues to not work with the internal module, I don't know what to say.
I suppose you're not using an X10S Express in your tests, but a Jumper T16 as master.
I ordered the internal module for the T16, but at the moment I can't test it as a master.

@raphaelcoeffic
Copy link
Member Author

I have the X9lite as well, which has an ISRM as well, so I can probably make it work with that to fix your issue.

@raphaelcoeffic
Copy link
Member Author

Issue with the 102.4% is now adjusted / fixed.

@kilrah
Copy link
Member

kilrah commented Oct 6, 2019

Nice! However it continues to not work with the internal module, I don't know what to say.

I've explained it doesn't work at boot but does if you turn the internal module off/on.
Reason will probably be found at some point and fixed.

I suppose you're not using an X10S Express in your tests,

X12S

@robustini
Copy link
Contributor

I've explained it doesn't work at boot but does if you turn the internal module off/on.

Already tried Andre, same issue also with the off/on trick, see my video.

@kilrah
Copy link
Member

kilrah commented Oct 6, 2019

Yeah becasue you have the radios stuck to each other, FrSky proto will swamp if you're not at least 1-2m away.

But you can see that "no multi_telemetry" disappears when you turn off internal.

@robustini
Copy link
Contributor

robustini commented Oct 6, 2019

Yeah becasue you have the radios stuck to each other, FrSky proto will swamp if you're not at least 1-2m away.

But you can see that "no multi_telemetry" disappears when you turn off internal.

Naaaa, already tried, the reason is not that.
So why does it work with the ISRM disabled and the radios almost glued?
It is not a classic rf saturation problem, i tried it three meters away now, same issue... damn! :-)

@lorenzing
Copy link

Hi, I'm trying too but i've exactly the same problem as @robustini , as soon as I activate the internal ISRM module the trainer stops working.
I use an X10 Express as master with IRangeX IRX4 multiprotocol module, and Taranis Plus as slave.
Thank you very much for your help, it's very important for me this function.

@3djc 3djc added the Incomplete label Oct 7, 2019
@robustini
Copy link
Contributor

Thanks @lorenzing for your report, so I'm not crazy.

@3djc
Copy link
Contributor

3djc commented Oct 7, 2019

I flagged it incomplete, so maybe it will be clearer that this is unmerged, work in progress, targeting a branch that is not even available a nightly yet

@raphaelcoeffic
Copy link
Member Author

raphaelcoeffic commented Oct 8, 2019

Ok, so it seems that the ISRM are preventing SPORT from being used by the external module (don't ask me why, they don't use it themselves...).

On my X9 lite, I removed to connection between ISRM and SPORT line, and it works like a charm: I can have the multi-module with telemetry working AND ISRM on.

Here is a picture from the mod:
IMG_7746

@robustini
Copy link
Contributor

robustini commented Oct 8, 2019

Great @raphaelcoeffic, investigating the X10S Express ACCESS I did too, but the hardware modification it's slightly different:

https://www.rcgroups.com/forums/showpost.php?p=42904121&postcount=13067

@robustini
Copy link
Contributor

robustini commented Oct 8, 2019

I flagged it incomplete, so maybe it will be clearer that this is unmerged, work in progress, targeting a branch that is not even available a nightly yet

I think after the latest news and several bench tests with excellent results i suggest to remove the "incomplete" flag, it works fine.
Obviously with ISRM modules it requires a hardware modification, different in almost all the tx.
I hope for a merge in 2.3.2, thanks!

@3djc
Copy link
Contributor

3djc commented Oct 9, 2019

Why on earth is ISRM "playing" with SPORT line ?? It does not use it when running, does not use it when flashing. I can't think of any legitimate reason for that

@robustini
Copy link
Contributor

Why on earth is ISRM "playing" with SPORT line ?? It does not use it when running, does not use it when flashing. I can't think of any legitimate reason for that

And you're not the only one who doesn't understand it.
But as I explained perhaps FrSky wanted to block the possibility of transmitting with the second module in particular conditions, but it is not clear in what and why.
That this thing blocked the multi-protocol trainer on my X10S Express and @raphaelcoeffic's X9 Lite is consolidated.

@raphaelcoeffic
Copy link
Member Author

I can't think of any legitimate reason for that

But there are so many others, not only the legitimate ones ;-)

@robustini
Copy link
Contributor

I can't think of any legitimate reason for that

But there are so many others, not only the legitimate ones ;-)

I agree! ;-)
In the X12S there is the same lock, so I don't understand how the multi-trainer works for @kilrah.
The contacts on the redline are short-circuited, as in my X10S Express (now I remove the pin here as well).

20191009_103020

@robustini
Copy link
Contributor

robustini commented Oct 14, 2019

@3djc please before merge could you fix the problem reported in the first two images I posted here related to Jumper T16 with the BT module installed?
It's a UI messages problem when choosing the type of trainer, information appears that has nothing to do with the trainer mode, thanks!

@kilrah
Copy link
Member

kilrah commented Oct 14, 2019

That's a translation issue, need an Italian-speaking dev to maintain the IT translations.

@robustini
Copy link
Contributor

That's a translation issue, need an Italian-speaking dev to maintain the IT translations.

The X10 target use the same IT language and there's no issue in that menu.
If you want an Italian-speaking dev i'm here.

@3djc
Copy link
Contributor

3djc commented Oct 15, 2019

That's a translation issue, need an Italian-speaking dev to maintain the IT translations.

The X10 target use the same IT language and there's no issue in that menu.
If you want an Italian-speaking dev i'm here.

It is not only Italian, will look at it

@3djc
Copy link
Contributor

3djc commented Oct 15, 2019

@robustini should be fixed, could you test ?

@robustini
Copy link
Contributor

@robustini should be fixed, could you test ?

Fixed, cool thanks!

@3djc 3djc removed the Incomplete label Oct 16, 2019
@robustini
Copy link
Contributor

@raphaelcoeffic it would be useful to have the RSSI on the slave tx, unfortunately there is not currently, in the firmware of the multi-protocol Pascal tells me that it is already implemented, so it is missing here.

@3djc
Copy link
Contributor

3djc commented Oct 18, 2019

That would mean the master multi has to send telem, which can swamp craft module doesn,'t sound like a good idea, or am I misunderstanding ?

@robustini
Copy link
Contributor

@raphaelcoeffic it would be useful to have the RSSI on the slave tx, unfortunately there is not currently, in the firmware of the multi-protocol Pascal tells me that it is already implemented, so it is missing here.

https://www.rcgroups.com/forums/showpost.php?p=42974047&postcount=15547

@raphaelcoeffic
Copy link
Member Author

Yes, there are some data we could display on the master. Just not clear how that could be displayed. We’ll figure something out, the data are not going away ;-)

@kilrah
Copy link
Member

kilrah commented Oct 18, 2019

@raphaelcoeffic
Copy link
Member Author

Guys, it’s quite funny to battle with quotes from RCG, but I’m not sure here is the place for that. Once you’re done in RCG, just write me a summary 😁

@robustini
Copy link
Contributor

I just said it would be nice to have this thing, not that it was possible, that Pascal said! :p

@3djc 3djc merged commit 6d77932 into 2.3 Oct 22, 2019
@3djc 3djc deleted the raphaelcoeffic/multi-trainer branch October 22, 2019 13:53
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

5 participants