EdgeTX / edgetx

EdgeTX is the cutting edge open source firmware for your R/C radio
https://edgetx.org
GNU General Public License v2.0
1.59k stars 338 forks source link

Signal polarity different to 2.8.1 for LemonDSP, DSM2 and PPM external modules #3413

Closed mha1 closed 1 year ago

mha1 commented 1 year ago

Is there an existing issue for this problem?

What part of EdgeTX is the focus of this bug?

Transmitter firmware

Current Behavior

The "polarity" of signals sent to external modules LemonDSP, DSM2 and PPM are reversed compared to 2.8.1.

Example DSM2 outgoing signal recorded at CCPM pin of the module bay with 2.8.1 firmware is idle high DSM2_2 8 1

with 2.9.1. the signal is idle low DSM2_2 9

The same is true for external modules LemonDSP and PPM. At least external DSM2 and LemonDSP modules shouldn't work in 2.9 as of now. I'd ask LemonDSP and DSM2 owners to confirm.

Expected Behavior

Same signal polarity as in 2.8.1

Steps To Reproduce

Version

Nightly (Please give date/commit below)

Transmitter

Radiomaster TX16S / TX16SMK2

Anything else?

Firmware versions used:

pfeerick commented 1 year ago

In case you check before I do, for PPM, does changing - to + still work (but basically be reversed to 2.8?)

mha1 commented 1 year ago

Yes, I checked this. +/- works but everything is reversed in 2.9. Same for SBUS normal/inverted.

pfeerick commented 1 year ago

Ok, so a walk down all the external modes, 2.9 yesterdays nightly vs 2.8.1, indicates:

Inverted:

Unchanged:

Broken?

Saleae Logic 8 captures: 3413-2.8.1.zip 3413-2.9.zip

mha1 commented 1 year ago

Correct, confirms my measurements.

Please do me a favor while you have the LA hooked up:

pfeerick commented 1 year ago

Sorry, no. Even when doing a 60 second capture and monitoring analog it still seemed clear. Using same 2.9 nightly as before. SBUS-60sec.zip (digital only)

mha1 commented 1 year ago

That's alright, just means less work.

mha1 commented 1 year ago

Not sure about external Flysky. It is initialized with .direction = ETX_Dir_TX_RX which looking at crossfire initialization seems to indicate half duplex (on SPort pin). So external Flysky should be ok with no signal on CCPM.

mha1 commented 1 year ago

Leaves PPM and DSM2 as LemonDSMP TX is also initialized as DSM2 (with different frame rate)

pfeerick commented 1 year ago

Except... this is FLYSKY on 2.8.1

image

vs 2.9 (and yes, a much wider time period to further highlight the point)

image

So either 2.8.1 was wrong, or 2.9 is wrong ;)

mha1 commented 1 year ago

My bad, you are right. 2.8.1 outputs serial data on CPPM for external Flysky, 2.9 doesn't as it seems to initialize the module port for half duplex on SPort. No clue what should be selected. Were there any complaints about external Flysky in 2.8.1? Isn't there a Flysky guy hanging out here? He's probably best to ask.

pfeerick commented 1 year ago

No issues that I know of... that is why I'm hoping it's expected change... Looking at you @richardclli šŸ¤­

mha1 commented 1 year ago

Do external AFHDS3 modules even exist? I only found 2a modules.

Edit: Found it: https://www.flysky-cn.com/frm302-canshu-1

says PPM/UART mode

gagarinlg commented 1 year ago

Do external AFHDS3 modules even exist? I only found 2a modules.

Patience

pfeerick commented 1 year ago

There's the FRM301, FRM302 and FRM303...

The FRM301 is the module used in the PL18 series transmitters The FRM302 is the JR micro bay module like you found I don't know much about the 303, other than IIRC it replaces the 302 and is very similar (protocol wise at least) to the IRM301 used internally in the EL18. Richard has been working on that. https://github.com/EdgeTX/edgetx/issues/2863

richardclli commented 1 year ago

No issues that I know of... that is why I'm hoping it's expected change... Looking at you @richardclli šŸ¤­

Some information for references:

  1. FRM302 is not supported since 2.8.0, as Raphael implemented the new protocol for the internal module IRM301 in EL18, the old protocol is removed completely. And that is why FRM302 will not work since 2.8.0
  2. Flysky has already discontinued FRM302, and they think it is alright to stop supporting FRM302
  3. FRM 303 will be announced soon, and FRM303 is already supported in EL18 since 2.8.1 and it did not use the S.Port for half duplex but use the PPM/Heartbeat pair for full duplex instead.
  4. Because of the hardware bug and forced signal inversion of other EdgeTX radios, the current implementation will not work for other radio, and we need to wait for Flysky to amend another mode that use S.Port instead.
pfeerick commented 1 year ago

Thanks for that :) Are you able to double-check that the FRM303 is still working with a recent nightly?

richardclli commented 1 year ago

Let me check during weekend. It works in main sometimes ago, but well who knows.

richardclli commented 1 year ago

Let me check during weekend. It works in main sometimes ago, but well who knows.

Confirmed FRM303 still works in 2.9 with EL18. Check with version 1f82724a9. So the signal is 2.8.1 is somewhat different with 2.9? They should be the same if it still works.

pfeerick commented 1 year ago

I was using the TX16S for the captures in https://github.com/EdgeTX/edgetx/issues/3413#issuecomment-1487936737, so there could be something different happening there. But as long as it works on the EL18, all good, as it won't work with the TX16S in that mode atm anyway šŸ¤­

richardclli commented 1 year ago

I see, TX16S use different mappings for Flysky now, and it is not working anyway. And why 2.8 has something, because I hhave tried to make it works in Tx16S, but not tried to do the same thing in 2.9, yet. The Serial API changes in 2.9 need different implementation.

pfeerick commented 1 year ago

Ah, that explains it... Thanks Richard :) As long as we know the reason, and it's not a šŸ›, the show can go on! šŸ˜„

richardclli commented 1 year ago

Now the FRM303 works in TX16S with a simple mod similar to Access mod. https://github.com/EdgeTX/edgetx/wiki/Flysky-FRM303-Mod-for-TX16S

pfeerick commented 1 year ago

I'm not sure atm if #3442 is connected to this issue, so re-opening for now.

gagarinlg commented 1 year ago

@mha1 could you please test https://github.com/EdgeTX/edgetx/pull/3513

mha1 commented 1 year ago

@gagarinlg Sure, but please be aware that this PR can't be verified with just a test on a single radio. The changes made are heavily influenced by the hardware design of the radios (UARTs, inverters, software controllable inverters, GPIO assignment, etc). So I can only contribute a Horus target (TX16s) but to come to a final verdict on this change you'd at least have to get results from NV14 (sw controllable inverters), and some Taranis type radios, e.g. X9D. See also discussion here https://github.com/EdgeTX/edgetx/pull/3415#issuecomment-1500848279

Test results on TX16s with firmware obtained from merge checks of this PR (https://github.com/EdgeTX/edgetx/actions/runs/4777343713?pr=3513):

DSM2 init (going from OFF to DSM2): ok image

DSM2 (for all subtypes LP45, DSM2, DSMX): polarity idle high as in 2.8 -> ok image

LemonRx DSMP init (going from OFF to LemonRx DSMP): ok image

LemonRx DSMP: polarity idle high as in 2.8 -> ok image

PPM (-) going from OFF to PPM -> not ok. This doesn't seem to hurt PPM modules as this only affects the first two or three frames. Yet, it doesn't look right. image

PPM (-): polarity idle high as in 2.8 -> ok image

PPM (+): polarity idle low as in 2.8 -> ok image

raphaelcoeffic commented 1 year ago

PPM (-) going from OFF to PPM -> not ok

I can re-check that one. And give it a try of side-by-side tests with X9D+.

mha1 commented 1 year ago

Thanks, and before merging we need at least feedback on NV14.