For SPEKTRUM telemetry, the sensor with 6 lipo cell provides wrong values for the last 5 cell voltages

[mstrens]

Is there an existing issue for this problem?

• I have searched the existing issues

What part of EdgeTX is the focus of this bug?

Transmitter firmware

Current Behavior

When the spektrum sensor (SRXL2) is filled with following data:
A6 80 16 21 3A 0 4 57 8 AE D 5 11 5C 7F FF 7F FF 7F FF F4 DC

The first cell should be reported as 11.11V based on 0X04 and 0X57. This is the case. So cell1 is OK
The other cells are reported with following values
cel2=677.58 (instead of 22.22 based on 0X08 0XAE)
cel3=1344.05 (instead of 33.33)
cel4=2010.52 (instead of 44.44)
cel5=2949.11 (for this the value is 7FFF which means that the value is not provided)
cel6=3604.47 (for this the value is 7FFF which means that the value is not provided)

Expected Behavior

Correct values for cell2...cell4;
Cell5 and 6 should not be displayed (because the value is 0X7FFF = not provided)

Steps To Reproduce

simulate the telemetry frame given above.

This has been tested with version 2.10.0 Rc1 Centurion

Version

Other (Please specify below)

Transmitter

RadioMaster TX16S / TX16SMK2

Operating System (OS)

No response

OS Version

No response

Anything else?

No response

[pfeerick]

@frankiearzu Since I know you just love Spektrum telemetry ... this for you (pretty please?)

[mstrens]

I had a short look at the code in edgeTx and in spektrum.cpp, I see:
else if (i2cAddress == I2C_CELLS && sensor->unit == UNIT_VOLTS) {
// Map to FrSky style cell values
int cellIndex = (sensor->startByte / 2) << 16;
value = value | cellIndex;
} // I2C_CELLS

The issue is that for e.g. cell2, startByte = 2. So cellIndex = 0X10000.
So value is 0X10000 | 0X08AE and this is 67758 in decimal what is the value displayed on the handset.

I did not check the rest of the code to see how the "value" is processed.

[mstrens]

I also notice another point of attention.
The doc from spektrum about his sensors (0X3A) says that each of the 6 cells is a uint16_t and that value should be 0x7FFF when the cell not present. This seems me inconsistent. Generally, when a uint16_t field is not present, the value should be 0XFFFF when the cell is not present.
It could be that this is just a bug in the documentation of spektrum.
For my tests, I did not used an official spektrum sensor but I simulate it using an openXsensor module. This module was based on the doc from spektrum and so is perhaps wrong about the value to use when cell is not present.
Note: I can easily change my code to fill the frame with 0XFFFF instead of 0X7FFF if this the value used by an "official" spektrum sensor.

[frankiearzu]

@mstrens I have found many inconsistencies on the use of NO-VALUE (0xFFFF or 0x7FFF) for the UINT type, what I have seen in the Spektrum TX is that they either fix it for individual cases or they treat it as INT (and not UINT), since is display correctly,

Maybe for this case, since the value stored is not that big (32767 = 327.67v), we could be set to INT instead of UINT, so that no extra code is needed to ignore the value. Or Add a new condition to the "if (i2cAddress == I2C_CELLS && sensor->unit == UNIT_VOLTS && value != 0x7FFF)"

From your example, the values are made up?? right?? Since is LIPO Cell, voltage should be around 4.20v MAX for each cell.

Cell1 = 0x4 0x57 = 1111 = 11.11v
Cell2 = 0x8 0xAE = 2222 = 22.22v
Cell3 = 0xD 0x5 = 3333 = 33.33v
Cell4 = 0x11 0x5C = 4444 = 44.44v
Cell5 = 0x7F 0xFF NO-DATA
Cell6 = 0x7F 0xFF NO-DATA

Still have to see how the value is processed for DISPLAY. Compared to the "Smart" Battery, looks like it uses the name is "Cels" for all the cells, but in Smart Batt the name changes for each ("Cel1, Cel2....").

[frankiearzu]

Looking more on the processing of the data, looks like there is a special UNIT_CELLS, is a 32bit value, the upper 16bits gives info about the CELL Index and #of Cells. and the lower 16bits is the value..

This is the Frsky processing code:

In Specktrum, we have it as UNIT_VOLTS, maybe changing it to UNIT_CELLS make it behave as expected. But also seems like we need to know the cell count. Probably we will need a custom function processing for the entire message, and not for each cell, but will give it a try first with just changing it to UNIT_CELLS.

[mstrens]

In Frsky format, there are 2 cells values (each in 3 bytes) in a field of uint32_t. and the MSB contains 2 informations (each in 4 bits).
One info is the number of cells and the other an index (0,2,4) to say if this 32bits contains cells1+2 or 3+4 or 5+6.

I expect that it is not so easy with the spektrum frame to build such a combined UINT32_t field.
Is it not easier to make a change in the table that defines the different fields in the frames. Instead of mapping each cell value to STR_SENSOR_CELLS, is it not enough to map them to different label like this:
// 0x3A Lipo 6s Monitor Cells
SS(I2C_CELLS, 0, uint16, STR_SENSOR_CL01, UNIT_VOLTS, 2), // Voltage across cell 1, .01V steps
SS(I2C_CELLS, 2, uint16, STR_SENSOR_CL02, UNIT_VOLTS, 2),
SS(I2C_CELLS, 4, uint16, STR_SENSOR_CL03, UNIT_VOLTS, 2),
SS(I2C_CELLS, 6, uint16, STR_SENSOR_CL04, UNIT_VOLTS, 2),
SS(I2C_CELLS, 8, uint16, STR_SENSOR_CL05, UNIT_VOLTS, 2),
SS(I2C_CELLS, 10, uint16, STR_SENSOR_CL06, UNIT_VOLTS, 2),
SS(I2C_CELLS, 12, uint16, STR_SENSOR_TEMP2, UNIT_CELSIUS, 2), // Temperature, 0.1C (0-655.34C)

This could also be done in the same way for another Spektrum frame that contains 16 cells values (each in a format uint8_t).

Note: I know that the cell values I used in my test (11.11 , 22.22, ...) are not realistic for lipo cells. In fact in the project oXs, I can measure 2X4 voltages that can be anything (so not only individual lipo cell). One of my friend want to get several voltages that can be e.g. 12V or more. As the voltages in the spektrum frame are define in uint16_t, it was in theory not an issue to use them to provide such high voltage. It is a "misused" of an existing spektrum frame.
If each cell value (uint16_t) from the spektrum frame is mapped to a different edgeTx field (STR_SENSOR_CL01, ...) I expect it should work. To be tested.

[frankiearzu]

Individual Cells names is the easier change.
Looking at the code for UNIT_CELLS, one of the advantages is to be able to calculate a sensor for the Lower, Higher and Difference between all the cells. Starting to change the code, but is time to go flying.. nice day here in Texas!! Will do a test tonight.

[mstrens]

Thank you.
Let me know if you finally use individual Cell names or keep UNIT_CELLS.
Please let me also know which value I should use when a voltage is not used (0X7FXX or 0XFFFF).
I do not have an original spektrum lipo sensor. So I do not know which code is the right one. I presume it should be 0XFFFF but I am not sure.

[pierrotm777]

Thanks for your last firmware witch work very well.
Just one thing, have a message in English each 10s "Missing Servoning" ?
It's normal ?

[pfeerick]

Maybe it's saying "sensor lost"? If it is saying it in English, are you using the English voice pack instead of the French one?

[pierrotm777]

No, all my sounds are in french :-)
Is it possible to stop this message by default ?

[pfeerick]

First thing will be to find out what message this is... this is the list of all the words/prasses for the french sound pack.. can you tell which one it is? There are no English phrases in that as far as I know, so it should not be saying anything in English ;) https://github.com/EdgeTX/edgetx-sdcard-sounds/blob/main/voices/fr-FR.csv

If it is saying sensor lost... did you delete all your telemetry sensors and do discovery with the new firmware? Or at least delete the old lipo cell sensor and do discovery again, as perhaps the old sensor is not valid? Either way, you probably need to find out why it is saying that, as it shouldn't be happening, and is telling you there is a fault that needs fixing.

[pierrotm777]

I don't understand why i listen to an English sentence with only french sounds on my sd card :-) .

I have try to delete all my sensor and discover them.

I have added a lipo with the same issue . I need to investigate more.

[pierrotm777]

I have found my issue witch isn't an EdgeTx problem but a widget mahRe2 issue .
I had forgotten to define the settings for this widget.
It's ok know.

Sorry :-)

[pfeerick]

Oh, good, glad you found the culprit :)

…

On Mon, 11 Mar 2024, 11:02 pm pierrotm777, ***@***.***> wrote: I have found my issue witch isn't an EdgeTx problem but a widget mahRe2 issue . I had forgotten to define the settings for this widget. It's ok know. Sorry :-) — Reply to this email directly, view it on GitHub <#4715 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABJ66KNDSHEWCPAFTPKXTZTYXWTOJAVCNFSM6AAAAABEO22GHKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBYGM4TCMBTGI> . You are receiving this because you commented.Message ID: ***@***.***>

[frankiearzu]

@mstrens @pierrotm777

Here is the 2.9.4 + the Lipo monitor code + fix from converting Temp from unit Celsius->Fahrenheit correctly (when it has decimals).

https://github.com/frankiearzu/DSMTools/blob/main/EdgeTx_Firmware/tx16s-edgetx-v2.9.4-Cell-fix.bin

[pierrotm777]

Thank you for your fantastic work ☺️