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.57k stars 334 forks source link

Many Spektrum telemetry sensors not defined. #3368

Closed mstrens closed 1 year ago

mstrens commented 1 year ago

Is there an existing issue for this feature request?

Is your feature request related to a problem?

On edgeTx handset , there are many telemetry sensors that are not displayed in the expected fields. Spektrum uses an ID for each type of sensor. Many of them are not yet defined in the table used by EdgeTx to recognise the different fields in the telemetry frame. I have e.g. following sensors that I can' use because they are not defined in the table

Describe the solution you'd like

Please fill the edge table to support: ////////////////////////////////////////////////////////////////////////////// // // GPS // Binary Type // // NOTE: Data resolution for all fields matches Crossfire EXCEPT speed. // ////////////////////////////////////////////////////////////////////////////// // typedef struct { uint8_t identifier; // Source device = 0x26 uint8_t sID; // Secondary ID uint16_t altitude; // m, 1000m offset int32_t latitude; // degree / 10,000,000 int32_t longitude; // degree / 10,000,000 uint16_t heading; // degree / 10 uint8_t groundSpeed; // km/h uint8_t numSats; // count } attribute((packed)) STRU_TELE_GPS_BINARY;

and also

////////////////////////////////////////////////////////////////////////////// // // RX CAPACITY METER // ////////////////////////////////////////////////////////////////////////////// // typedef struct { uint8_t identifier; // Source device = 0x18 uint8_t sID; // Secondary ID int16_t current_A; // Instantaneous current, 0.01A (0-328.7A) 7FFF-> no data uint16_t chargeUsed_A; // Integrated mAh used, 0.1mAh (0-3276.6mAh) uint16_t volts_A; // Volts, 0.01V increments (0-16.00V) int16_t current_B; // Instantaneous current, 0.01A (0-328.7A) 7FFF-> no data/sensor B uint16_t chargeUsed_B; // Integrated mAh used, 0.1mAh (0-3276.6mAh) uint16_t volts_B; // Volts, 0.01V increments (0-16.00V) uint8_t alerts, // Bit mapped alert conditions (see below) highCharge; // High nybble is extra bits for chargeUsed_B, Low is for chargeUsed_A } attribute((packed)) STRU_TELE_RX_MAH;

define RXMAH_PS_ALERT_NONE (0) // No alarms

define RXMAH_PS_ALERT_RF_INT (1 << 0) // A or internal Remote failure

define RXMAH_PS_ALERT_RF_ANT1 (1 << 1) // B remote power fault

define RXMAH_PS_ALERT_RF_ANT2 (1 << 2) // L remote power fault

define RXMAH_PS_ALERT_RF_ANT3 (1 << 3) // R remote power fault

define RXMAH_PS_ALERT_OVERVOLT_A (1 << 4) // Battery A is over voltage

define RXMAH_PS_ALERT_OVERVOLT_B (1 << 5) // Battery A is over voltage

define RXMAH_PS_ALERT_RFU1 (1 << 6)

define RXMAH_PS_ALERT_RFU2 (1 << 7)

Describe alternatives you've considered

No response

Additional context

The table to complete in EdgeTx is: spektrumSensors[] (in file spektrum.cpp).

The list of all sensors defined by spektrum is in this document: https://github.com/SpektrumRC/SpektrumDocumentation/blob/master/Telemetry/spektrumTelemetrySensors.h

frankiearzu commented 1 year ago

Have been working on the spectrum telemetry lately. Specifically the GSP, ID=0x16 and 0x17 (non binary), and AS3X/SAFE Flight Mode.

The challenge is getting a sensor to test. Do you own this sensors???

I have a way that we can capture the data and save it into a file in your radio, and then in development (without the sensor), i can use some of the capture data to develop/test locally.

If you can capture some sample data, that will help a lot.

Check this other similar issue that we are trying to fix it using this remote data capture method: https://github.com/EdgeTX/edgetx/issues/3290

mstrens commented 1 year ago

In fact I am working an a project named openXsensor on RP2040. This project emulates different kinds of sensors (GPS, Vario, Volt, Current, RPM, Temp) for different Rc protocols (Sport, Jeti, Fbus, SBUS2, Ibus, Hott, SRXL2). I do not have spektrum device but a friend has a spektrum receiver and a radiomaster TX with EdgeTX. I develop the code and he test it.

For SRXL2, I did not planned to develop 16 and 17 frames for GPS because it seems me much better to use the binary format (= 0X26 frame). The code is written for vario and tests are OK. My friend can probably do what you want for the 0X26 and 0X18. I expect that for those sensors, there is no development required on EdgeTX but it "only" require to fill one table.

raphaelcoeffic commented 1 year ago

@mstrens hi! Nice to see you here! I’ve been using openXsensor in quite some planes ;-) feel free to just file a PR if you need anything, that is much easier as you can get it tested as well ;-)

mstrens commented 1 year ago

I never installed Edge on my Tx. I am still with openTx. I asked my fiend if he could compile and test with his TX. He installed version 2.8 on his TX but I presume he had just to it in a list and did not had to compile it. I wait his answer.

I could propose some changes to Edge for spektrum telemetry but I can't test it. I already saw 5 bugs for spektrum ESC telemetry.

frankiearzu commented 1 year ago

Nice to meet you @mstrens! whats your name? I haven't seen much development progress in OpenTx lately.. EdgeTX is way more active.

Would be great to coordinate and collaborate, and let me know more about your sensors.. would like to use them.

I think my little tools here can help more development.

  1. Have a LUA script tool called DSM_Capture_Tel.lua. This can capture any Spektrum telemetry stream ID and save it into a file in the radio. Currently only ID 0x16 and 0x17 but can be easilly changed to capute others. This captures can help you see how the spektrum sensors sends data and get samples.

  2. Have other LUA script called: DSM_Smart_Tel.lua, who can show a lot of telemetry infrom from Spektrum. Currently using some of the ogly automatic generated names (like 0502; ID=0x5, word=2) some some who are not processed by the Telemetry code.

  3. Added some testing code to "spektrum.cpp" where it can replace an incomming telemery frame, and replace it with the caputured one. Currently replace ID 0x05 (AS3X/SAFE flight controller) for the sensor that i want to develop/test. In that way, I can do the EdgeTX development without having the hardware available. Then for production, just turn OFF the testing flag to submit the PR code to edgeTX. Also found that any BDC data was processed totally wrong (even single byte data), so all the JetCats sensors was never proprtly tested. Check the new code for spektrum.cpp: https://github.com/EdgeTX/edgetx/pull/3366

  4. Sensor Table: as you said, some sensors are easy to add if it does not need any transformation, just fill the table. Otherwise you will need some extra code to handle them later. GPS and DATE needs two calls to set value for (LAT/LON), and (DATE/TIME).

  5. I have a github repo with some of the tools im developing: https://github.com/frankiearzu/DSMTools

Bye, Frankie

raphaelcoeffic commented 1 year ago

@mstrens let me know in case you need a second radio to be able to test with EdgeTx. Just contact me on Discord: drop by at the dev channel and ask for Raphael.

mstrens commented 1 year ago

I prepare some changes to the file spektrum.cpp. I added the comment "mstrens" to the lines that are modified. Changes are about:

spektrum.zip

mstrens commented 1 year ago

On rcgroups forum, there is a message saying that all Spektrum sensors are nor using big-endian in the telemetry frame. If it is so, there is an issue in edgeTx because EdgeTx assumes that all fields are big-endian. I asked which sensors use big-endian and which use little-indian but I did not (yet) got the answer.

frankiearzu commented 1 year ago

It can handle both big-endian and little-endian.. you just have to change the type in the table. By default, big-endian. But there are two specialized types for little (only for unsigned numbers), if we need signed, we can add them (int16le, int32le)

We need to know what sensors uses big or little. We could capture the frames from someone who has the sensor, so we can be sure, as well as use those frames for testing.

------- Data types ---------------------- enum SpektrumDataType : uint8_t { int8, int16, int32, uint8, uint16, uint32, uint8bcd, uint16bcd, uint32bcd, uint16le, <====== Little Enddian uint32le, custom };

frankiearzu commented 1 year ago

@mstrens, will do your changes and compile it for your to test. Will have some time over the weekend. What radio are you using?? TX16S?

mstrens commented 1 year ago

Thanks. I do not have a radio with EdgeTX but it is a friend that makes the tests with his radio. I think it is a radiomaster TX16s. I asked him to confirm here in this topic.

pierrotm777 commented 1 year ago

Hi frankiearzu, I help mstrens on it's project oXs RP2040. I have a Radiomaster Tx16s MkII 4IN1 version. Actually I use the 2.8.1 version for oxs tests. When your Spektrum DSMX adds will be done, if you build a firmware for my transmitter that will be easier for me. Thanks for your help, Pierre

frankiearzu commented 1 year ago

@mstrens @pierrotm777

Firmware with your changes.. based on 2.8.1 I don't see different compilations options between TX16S and TX16S MkII. https://github.com/frankiearzu/DSMTools/blob/main/EdgeTx_Firmware/edgetx_TX16S_2.8.1_mstrens.bin

How spektrum.cpp looks: spektrum.zip

Let me know if your binary GPS works now, as well as the fixes for ESC values.

frankiearzu commented 1 year ago

@pierrotm777 wrote:

I can see now the gps location on only one line but it return me bad value. For : GPS Latitude = 89.1234560 degree GPS Longitude = 178.1234560 degree I obtain: 891°14'4.4''N et 1781°14'4.4''E

Did you have done the changes asked by mstrens ? If yes, where are the source with these change please ? Another thing, i discover always ID from 1800 to 180D I think it's why 0x18 data are omited.

We see also an error with the GAlt value that stay always -1000m.

@pierrotm777 Changes sugested by @mstrens are in the file spektrum.zip (see previous message), corrected to compile properly. @mstrens did not provide any code for 0x18.

So i guess the Lat/Long needs to be devided by 10. use the NMEA format, will be easier (decimal minutes) instead of DMS. The Altitude as adjusted with a -1000 offset as sugested by @mstrens code. Probably remove that adjustment. Also @mstrens sugested changes for the ESC rpm, and current.

mstrens commented 1 year ago

About GPS alt, I found the bug in my oXs program (value was not filled correctly).

About GPS long and lat, I think the error (adding a division by 10) is the spektrum.cpp program because the spektrum doc says that it is in degree with 7 decimals ("// degree / 10,000,000") and this is the way oXs fill the fields.

About sensor 0X18, here are the lines to be added // 0x18 Rx MAH dual {I2C_RX_MAH , 0, int16, STR_SENSOR_MAH_CURRENT_A, UNIT_AMPS, 2}, //Instantaneous current, 0.01A (0-328.7A) 7FFF-> no data {I2C_RX_MAH , 2, uint16, STR_SENSOR_MAH_USED_A, UNIT_MAH, 1}, // Integrated mAh used, 0.1mAh (0-3276.6mAh) {I2C_RX_MAH , 4, uint16, STR_SENSOR_MAH_VOLT_A, UNIT_VOLTS, 2}, // Volts, 0.01V increments (0-16.00V) {I2C_RX_MAH , 6, int16, STR_SENSOR_MAH_CURRENT_B, UNIT_AMPS, 2}, //Instantaneous current, 0.01A (0-328.7A) 7FFF-> no data {I2C_RX_MAH , 8, uint16, STR_SENSOR_MAH_USED_B, UNIT_MAH, 1}, // Integrated mAh used, 0.1mAh (0-3276.6mAh) {I2C_RX_MAH , 10, uint16, STR_SENSOR_MAH_VOLT_B, UNIT_VOLTS, 2}, // Volts, 0.01V increments (0-16.00V)

Still this creates new codes for names. So those codes should be added to another table in another file (code => string name)

mstrens commented 1 year ago

I forgot to say that all other changes seems OK.

frankiearzu commented 1 year ago

New firmare with your changes. Attached here the firware and the specktrum.cpp.

Don't know what name to give the new sensorts. Do you know what is the name on a spektrum radio??

// 0x18 RX CAPACITY METER {I2C_RX_MAH, 0, int16, "RXA_Curr", UNIT_AMPS, 2}, //Instantaneous current, 0.01A (0-328.7A) 7FFF-> no data {I2C_RX_MAH, 2, uint16, "RXA_Used", UNIT_MAH, 1}, // Integrated mAh used, 0.1mAh (0-3276.6mAh) {I2C_RX_MAH, 4, uint16, "RXA_Volts", UNIT_VOLTS, 2}, // Volts, 0.01V increments (0-16.00V) {I2C_RX_MAH, 6, int16, "RXB_Curr", UNIT_AMPS, 2}, //Instantaneous current, 0.01A (0-328.7A) 7FFF-> no data {I2C_RX_MAH, 8, uint16, "RXB_Used", UNIT_MAH, 1}, // Integrated mAh used, 0.1mAh (0-3276.6mAh) {I2C_RX_MAH, 10, uint16, "RXB_Volts", UNIT_VOLTS, 2}, // Volts, 0.01V increments (0-16.00V)

spektrum-April-2.zip edgetx_TX16S_2.8.1_mstrens2.bin.zip

pierrotm777 commented 1 year ago

Thank you Frank, I try this new version this morning. Pierre

frankiearzu commented 1 year ago

Was able to validate the RPM canculation (Erpm), as well as the current ECUR.. have to change it from mAh to A.. on a bigger plane using almost 80A, 80A is easier to read than 80000.0mAh. They match the telemetry using an Spektrum TX.

After doing your build, i continued testing, and fixed a few other minor things...

The EThr (Throttle percent) i also fix it to be 0-100% instrad of 0.0-10.0%. (no decimals). Tested also with a small Blade 230s V2 heli... can't belive that motor is going at 53k RPM at 50% power!.. and the Amps for the ESC (ECurr) are close to the AMPS supplied by the Battery (BCurr).

Also found another funny bug... telemetry connection starts sending data, but in the initial part, it uses i2CAddress=0 (no-op/dummy)... so if you are discovering sensors in EdgeTX, will create all the fake sensorts 0000,0002,0004, etc..

pierrotm777 commented 1 year ago

Also found another funny bug... telemetry connection starts sending data, but in the initial part, it uses i2CAddress=0 (no-op/dummy)... so if you are discovering sensors in EdgeTX, will create all the fake sensorts 0000,0002,0004, etc..

In fact, i have found sensors from 0000 to 000D :-)

frankiearzu commented 1 year ago

@mstrens @pierrotm777: Any news with the latest changes?

pierrotm777 commented 1 year ago

Hi Franki, all is OK now accept GSpd value that seems not correct. I have always the Id 0000 to 000D. If you can solve these two things that would be cool. Thanks in advance, Pierre

frankiearzu commented 1 year ago

For the ID 0000 to 0000D... have a fix for that.

On the GSpd is it a scaling problem (multiply or divide by 10)??? of the value does not make sense?? There is not much that we are doing in the code.. is just a plain uint8 (0..256 kmh)

Have to make nicer names for the sensors... (RXB_Curr, etc)

Will work this weekend to cleanup the code, and compile a new testing firmware.

pierrotm777 commented 1 year ago

For GSpd it's a value that is not much sense. I have 133kmh, without gps movement. Thanks again for your job. I wait your new firmware for last test. Cordially, Pierre

mstrens commented 1 year ago

For GPS speed, oXs transmits the value in km/h as defined in the doc from Spektrum.. Could it be that Edge expect the value in other units. I know that some protocols send the GPS in kts. This could perhaps explain that the value sent by oXs is not the same as the value being displayed on the TX.

pierrotm777 commented 1 year ago

Hello,

I test the firmware 2.8.3 with the Spektrum protocol.

ID 0000 to 000D now don't exits. But i discover now some ID from 2600 to 260C . I don't see GPS values :-( !

frankiearzu commented 1 year ago

2.8.3 don’t have your changes for binary GPS.We need to do a few more testing and get proper names for the battery sensors.I can create a 2.8.3 with that changes to continue that work.Frankie On Apr 22, 2023, at 10:58 AM, pierrotm777 @.***> wrote: Hello, I test the firmware 2.8.3 with the Spektrum protocol. ID 0000 to 000D now don't exits. But i discover now some ID from 2600 to 260C . I don't see GPS values :-( !

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you commented.Message ID: @.***>

pierrotm777 commented 1 year ago

Ok, I wait you new 2.8.3 new version for test 👌

pierrotm777 commented 1 year ago

Hi Franki, Does it possible you compile a new firmware 2.8.3 with last Spektrum fixed code? Thanks, Pierre

frankiearzu commented 1 year ago

@pierrotm777 2.9 will be released soon... this fix/improvement will be released. I can make a 2.8.4+this changes if you need them sooner. Traveling from Germany->USA today (Sat), but can take care of that on sunday,

pierrotm777 commented 1 year ago

Thanks Franki, as you want, if you have some time for build a 2.8.4 version, that will be cool. Cordially, Pierre

frankiearzu commented 1 year ago

@mstrens @pierrotm777 Recently got a genuine Spektrum flight pack energy sensor (ID 0x34) to debug issue #1649.
According to the Spektrum reference, sensors should use big-endian, but this one was really using little endian.

you posted earlier: "On rcgroups forum, there is a message saying that all Spektrum sensors are nor using big-endian in the telemetry frame. I asked which sensors use big-endian and which use little-indian but I did not (yet) got the answer."

I think this might be right, since the Flight Pack Energy sensor was sending data in little-endian, is manufactured by "Eagle Tree". I think all Eagle Tree might be little endians. The spektrum Altimeter and Avian ESC seems correctly using big-endian.

For the Binary GPS and RX capacity metter, i think they probably use little-endian too if they are manufactured by Eagle Tree. Do you have a genuine spektrum sensor? or is the one you develop the code?? in that case, we might need to change in both sides to little-endian if they are manufactured by Eagle Tree.

Frankie

crwardlaw commented 4 days ago

@frankiearzu I can create a seperate issue to take this forward, but my query relates to a similar issue. I have a kingtech turbine telemetry unit which kiddybacks on spektrum's jetcat telemetry interface when transmitting telemetry to a spektrum receiver. However, as can be seen in https://github.com/EdgeTX/edgetx/blob/149e1a2df00f680404691dadbb59de62abecc3bf/radio/src/telemetry/spektrum.cpp#L193 the turbine status implementation is still a TODO. Do you know if there's been any focus on this? When trying to utilise it tonight, on my TX16S MAX I, was able to detect the non-commented lines in the I2C_JETCAT address but not the status messages, they wouldn't even read as an int for example. The status message enums can be found in 2 sections of this file https://github.com/SpektrumRC/SpektrumDocumentation/blob/2a8a5f9d6f85eff0d2458b9ac2b05e9dcf7ddcab/Telemetry/spektrumTelemetrySensors.h#L739.

BUT kingtech use different phrases in their status enums from the jetcat enums which means there needs to be a way to specify the turbine being used, too. The differences can be found on page 41 in section 2.9.4 of https://drive.google.com/file/d/1TfaMCA3zpDcdGQMnRq5MnjJexTBp-VZD/view

frankiearzu commented 4 days ago

Really not much focus on turbine, since is not that common, but can be added.. thats what open source is for :-)

We should start a new improvement request. If i understand correctly, you are getting some data into sensors, but not all of them (Status for example). So if you are using a Spektrum TX, you will see the JetCat equivalent instead of KingTech. From the telemetry, unless something in the data that we can use to differenciate between a JetCat and Kingtech, the code will not have a way to process it differently.

We could aproach it in three ways:

  1. Just populate the status with its numeric value, and have a specialized "value" widget to display the equivalent text for that numeric status.. like a "valueKTEnum" widget or "valueJCEnum" widget.
  2. create separate sensors for the KT and JC status, populating them with the proper translation. So you pick wich one to use on the screen.
  3. Use a specialize widget to display a telemetry screen with all the tubine info together (similar to the spektrum telemetry screen).. the translation of the numeric status is done there.. so one wiget for KT, and one widget for JC.

Options 1/2 will allow you to log the value into telemetry logs... option 3 don't log, but probably looks nicer, and can be done without touching the firmware code.

Please create the new request, and we can work on what could be a good solution.

crwardlaw commented 3 days ago

Thanks for the prompt response Frankie - that's great to hear! My initial thought is that option 2 is the better as it keeps all the sensor data importing as close to the sensors page as possible, otherwise a new user would have to know separately about the widgets to translate it.

I'll make the request now!