search

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.64k stars 345 forks source link

Feeding telemetry via AUX1/AUX2 (TX16S) does not work anymore #4633

Open wimalopaan opened 9 months ago

wimalopaan commented 9 months ago

Is there an existing issue for this problem?

What part of EdgeTX is the focus of this bug?

Transmitter firmware

Current Behavior

Setting AUX1 to "telemetry-in" and sending serial telemetry-data does not provide new sensors in sensor search.

Expected Behavior

Sending FrSky-D telemetry-data via AUX1@9600Bd should produce new sensors in telemetry.

Steps To Reproduce

Set AUX1 to telemetry-in. Send FrSky-D telemetry-data 9600Bd to AUX1 RX. Search for new Sensors.

Version

Nightly (Please give date/commit below)

Transmitter

RadioMaster TX16S / TX16SMK2

Operating System (OS)

No response

OS Version

No response

Anything else?

No response

wimalopaan commented 9 months ago

The firmware on my external µC was working I think at least until 2.8.x and remains unchanged, but the radio stops recognizing the telemetry data the µC sends now. Maybe the FrSkyD-packets it sends were wrong from the beginning ... to help check this out, I append a serial packet view form the LA.

frskyd

Maybe #4632 has some influence on this.

hth

wimalopaan commented 9 months ago

Ok, the first question is, what the motivation was to introduce this check:

https://github.com/EdgeTX/edgetx/commit/ff2b3ac22ab3756204066153cd5fc2ac08212026#diff-c4a06aa2127e48201e600ecfb1fd4805378bc8b6d532adc93244f54a26589f48

This enables telemetry-in only when external module is ppm. That doesn't make sense to me.

pfeerick commented 7 months ago

This enables telemetry-in only when external module is ppm. That doesn't make sense to me.

Does it though (not make sense)? i.e. the only times I have ever used FrskyD (or at least I think it was FrskyD) telemetry in were in my 9XR days, to feed in telemetry from the XJT module. What are normal use cases are there? From my quick refresher of that block of code - and it's predecessors - this is not particularly new... it seems to have always been linked to the module being of PPM and a telemetry UART being available in some way or another, probably since this was the intended use case. :shrug:

That change you reference was part of https://github.com/EdgeTX/edgetx/pull/3822, so only affects 2.10, so if 2.9 is affected also that isn't the issue. I have this feeling your other problem here (also 2.10 specific) is that g_model.telemetryProtocol can't be set to PROTOCOL_TELEMETRY_FRSKY_D_SECONDARY via the UI... I don't think it was present on colorlcd UI at all for us, and this PR removed it from B&W and Companion for consistency... with the plan going forward mentioned there https://github.com/EdgeTX/edgetx/pull/3782#issuecomment-1676028946 ... meaning you might be the one to make that happen :grin:

wimalopaan commented 7 months ago

Thank you for your explanation!

The first question is: what is the purpose of PROTOCOL_TELEMETRY_FRSKY_D_SECONDARY ? And what is PROTOCOL_TELEMETRY_FRSKY_D instead?

If I grep the sources, I get:

$ grep -R PROTOCOL_TELEMETRY_FRSKY_D_SECONDARY * 2>/dev/null
radio/src/serial.cpp:        g_model.telemetryProtocol == PROTOCOL_TELEMETRY_FRSKY_D_SECONDARY) {
radio/src/dataconstants.h:  PROTOCOL_TELEMETRY_FRSKY_D_SECONDARY,

And here all seems to be disabled: https://github.com/EdgeTX/edgetx/blob/fb45009fd430a2fa83bcf35b9a2c412730ead589/radio/src/serial.cpp#L225-L231

Maybe you can guide me into the right direction? Is there kind of blueprint to implement a new serial telemetry stream?

pfeerick commented 7 months ago

You need to be looking at older code to find more references, say 2.9 branch, since the aforementioned PRs effectively removed any remaining traces of it but those.

Not as yet, it is still very much at formulation/discussion level.

On Tue, 9 Apr 2024, 7:49 pm Wilhelm, @.***> wrote:

Thank you for your explanation!

The first question is: what is the purpose of PROTOCOL_TELEMETRY_FRSKY_D_SECONDARY ? And what is PROTOCOL_TELEMETRY_FRSKY_D instead?

If I grep the sources, I get:

$ grep -R PROTOCOL_TELEMETRY_FRSKY_D_SECONDARY * 2>/dev/null radio/src/serial.cpp: g_model.telemetryProtocol == PROTOCOL_TELEMETRY_FRSKY_D_SECONDARY) { radio/src/dataconstants.h: PROTOCOL_TELEMETRY_FRSKY_D_SECONDARY,

And here all seems to be disabled:

https://github.com/EdgeTX/edgetx/blob/fb45009fd430a2fa83bcf35b9a2c412730ead589/radio/src/serial.cpp#L225-L231

Maybe you can guide me into the right direction? Is there kind of blueprint to implement a new serial telemetry stream?

— Reply to this email directly, view it on GitHub https://github.com/EdgeTX/edgetx/issues/4633#issuecomment-2044599005, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJ66KNMEAUE2GN3JFY5MM3Y4O2QZAVCNFSM6AAAAABDGQCIDOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBUGU4TSMBQGU . You are receiving this because you commented.Message ID: @.***>

pfeerick commented 7 months ago

Actually, sorry, yes, check the linked comment from before, Michael did say his PR (think it was #3352?) could be used à a template?

On Wed, 10 Apr 2024, 6:27 am Peter Feerick, @.***> wrote:

You need to be looking at older code to find more references, say 2.9 branch, since the aforementioned PRs effectively removed any remaining traces of it but those.

Not as yet, it is still very much at formulation/discussion level.

On Tue, 9 Apr 2024, 7:49 pm Wilhelm, @.***> wrote:

Thank you for your explanation!

The first question is: what is the purpose of PROTOCOL_TELEMETRY_FRSKY_D_SECONDARY ? And what is PROTOCOL_TELEMETRY_FRSKY_D instead?

If I grep the sources, I get:

$ grep -R PROTOCOL_TELEMETRY_FRSKY_D_SECONDARY * 2>/dev/null radio/src/serial.cpp: g_model.telemetryProtocol == PROTOCOL_TELEMETRY_FRSKY_D_SECONDARY) { radio/src/dataconstants.h: PROTOCOL_TELEMETRY_FRSKY_D_SECONDARY,

And here all seems to be disabled:

https://github.com/EdgeTX/edgetx/blob/fb45009fd430a2fa83bcf35b9a2c412730ead589/radio/src/serial.cpp#L225-L231

Maybe you can guide me into the right direction? Is there kind of blueprint to implement a new serial telemetry stream?

— Reply to this email directly, view it on GitHub https://github.com/EdgeTX/edgetx/issues/4633#issuecomment-2044599005, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJ66KNMEAUE2GN3JFY5MM3Y4O2QZAVCNFSM6AAAAABDGQCIDOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBUGU4TSMBQGU . You are receiving this because you commented.Message ID: @.***>

raphaelcoeffic commented 7 months ago

what is the purpose of PROTOCOL_TELEMETRY_FRSKY_D_SECONDARY ? And what is PROTOCOL_TELEMETRY_FRSKY_D instead?

AFAIK, it was to be able to support some DIY Multiplex module which was outputting its telemetry into the S.PORT pin in this format.

@3djc what does your memory tell you?

3djc commented 7 months ago

Yeah, some DIY stuff where you would out PPM and get back D telem, but this was way back then, unsure about brand tho

wimalopaan commented 7 months ago

AFAIK, it was to be able to support some DIY Multiplex module which was outputting its telemetry into the S.PORT pin in this format.

Ok, sounds really that I just picked something that wasn't really in use ;-)

There is still https://github.com/EdgeTX/edgetx/pull/4102

Unfortunately I still had not the time to finalize this, but maybe we can include FrSky-D there. But on the other hand, since at present it seems that I am the only one who used that, it could be fully dropped.

My use case was connecting a small attiny1614 based board, that encodes 4 stick-switches into some telemetry-value and this was read via AUX2. The attiny gets its supply also from AUX2, so turning of the supply of AUX2 turns off the attiny and frees AUX2 for other usages.

The same can be achieved via CRSF or SumdV3.

Comments?

raphaelcoeffic commented 7 months ago

My use case was connecting a small attiny1614 based board, that encodes 4 stick-switches into some telemetry-value and this was read via AUX2. The attiny gets its supply also from AUX2, so turning of the supply of AUX2 turns off the attiny and frees AUX2 for other usages.

Wouldn't it make way more sense to have you board using some "real" input extension protocol? Having a way to add inputs to the radio seems like a desirable feature, which could be done properly instead of using some hacky way via telemetry.

wimalopaan commented 7 months ago

some "real" input extension protocol

Do you have special in mind?

3djc commented 7 months ago

Wouldn’t trainer be a better fit than telemetry to get extra input in?

wimalopaan commented 7 months ago

Wouldn’t trainer be a better fit than telemetry to get extra input in?

It would be cool to have proportional external input as well as binary / ternary (or arbitrary N-ary) "switch"-values (with the possibility to handle them in EdgeTx).

SBus (as well as may IBus) serves as trainer input but lacks the "switch" values. SumDV3 can transport only binary switches. CRSF could / must be extended, e.g. https://github.com/crsf-wg/crsf/issues/12

raphaelcoeffic commented 7 months ago

Do you have something special in mind?

Well, something that would be closer to the hardware, and thus allowing us to integrate at lower level, and thus be usable in many flexible ways.

Injecting values via trainer protocol is a bit cumbersome to integrate. I'm thinking about something that would allow a more "built-in" feeling, more like some extension that would allow custom input types, as if they were built-in into the radio. Think additional switches, pots, gimbals, etc.

wimalopaan commented 7 months ago

Well, something that would be closer to the hardware, and thus allowing us to integrate at lower level, and thus be usable in many flexible ways.

Do you mean also another physical layer, e.g. I2C? But that isn't availabe at AUX1/2 I think.

Injecting values via trainer protocol is a bit cumbersome to integrate.

Why? Which obstacles do you see?

I'm thinking about something that would allow a more "built-in" feeling, more like some extension that would allow custom input types, as if they were built-in into the radio. Think additional switches, pots, gimbals, etc.

What I meant was

It would be cool to have proportional external input as well as binary / ternary (or arbitrary N-ary) "switch"-values (with the possibility to handle them in EdgeTx).

raphaelcoeffic commented 7 months ago

Do you mean also another physical layer, e.g. I2C? But that isn't available at AUX1/2 I think.

Well, something similar, where the device would send the values/positions of it's different hardware inputs. And yes, that would require some more integration. And by the way, I2C can be used on AUX1 instead of USART, at least on the TX16S (done this before).

Why? Which obstacles do you see?

Instead of just settings up switches and using them wherever you like, you end up settings up a bunch of logical switches first that all depend on trainer channel values. Also, what if you want to actually use a real trainer input (from another radio), but still use your additional inputs?

rotorman commented 7 months ago

For reference, here the original IMU PR that turned TX16S AUX1 from serial into I2C: https://github.com/EdgeTX/edgetx/pull/617

wimalopaan commented 7 months ago

Well, I don't think I2C would be a good idea here, because

For me it would be more practical to stick to UART and define some protocol that fullfills the requirements (as I stated above).

In EdgeTx the trainer proportional channels can act as normal inputs (TR1-TR16)

For the switches we then must extend the datastructure of switches (binary, ternary, ..., 6pos) in EdgeTx to those that come via the new protocol from extern.

rotorman commented 7 months ago

How about MAVLink as the "UART protocol"? The channel data could be transmitted with https://mavlink.io/en/messages/common.html#RC_CHANNELS and an aux of https://mavlink.io/en/messages/common.html#MANUAL_CONTROL could be mapped to various switches.

wimalopaan commented 7 months ago

How about MAVLink as the "UART protocol"?

Would be an option, but MAVLink is huge

rotorman commented 7 months ago

Yes, but the code is there: https://mavlink.io/en/getting_started/use_libraries.html (here especially https://github.com/olliw42/fastmavlink would likely be the one to go with). Especially the integrated MAVLink router in fastmavlink lib could be IMO of great interest here for you in regards to expandability.

wimalopaan commented 7 months ago

Yes, but the code is there: https://mavlink.io/en/getting_started/use_libraries.html (here especially https://github.com/olliw42/fastmavlink would likely be the one to go with). Especially the integrated MAVLink router in fastmavlink lib could be IMO of great interest here for you in regards to expandability.

Ok, need to have a closer look at that. Thanks.

BTW: what about olliw's work integration into edgetx?

rotorman commented 7 months ago

Here the "latest" state for EdgeTX: https://github.com/EdgeTX/edgetx/pull/150/files For truly latest state, see Olli's own repo (committs on top of OpenTX): https://github.com/olliw42/mTX

gagarinlg commented 7 months ago

Olli is not interested in working with us. He did not like something that happened a while ago. I made an offer work together, but I did not get any answer.

wimalopaan commented 7 months ago

What about MultiWii (MSP). Maybe a bit more widespread?

wimalopaan commented 7 months ago

Thinking a bit more about this topic I got the impression, that MAVLink is not the right protocol for extending the built-in pots/switches/... of a handset.

There are two reasons:

Extending the handset with external interface-elements (pots, switches) is far more low-level. But - as I said - should not be so low-level as I2C (register level).

Therefore I would like to propose a serial handset-extension-protocol. This should be as easy as SBus, IBus, SUmDV3, CRSF (only rc-channels), and it should transport the (normalized) values of external pots, incrementals, ADC-channels, switches (N-ary), buttons, etc. I think a uni-directional protocol would suffice.

I could be as simple as

<start> <len> <type> <payload> <crc>

where <type> could be

where <payload> is the data according to <type>.

Maybe we could finalize the work on #4102 and add a parser for this.

Additionally, to be of full use, it requires to extend the list of sources(the 16 external proportional) and switches (64). For the sources we could reuse the trainer-channel values: But it would be a bit more clearer to introduce new data-structures for these external sources and switches.

If the values of these external interface-elements are not used in inputs/mixer-lines they should be queryable via LUA. Then a LUA skript can forward them (crsftelemetrypush()) to a remote device (the FC or something behind the FC).