[LUA API] getValue() implementation

[JimB40]

getValue() regarding telemetry sensors is quite cumbersome as it returns 0 when:

value current source value (number). Zero is returned for: non-existing sources for all telemetry source when the telemetry stream is not received far all non allowed sensors while FAI MODE is active

As telemetry sensor is treated as any other available radio field, getting value of not discovered sensor will return 0. That's wrong IMHO as getting value of not discovered sensor (non-existing source) should return nil Now prior to getting sensor's value we have to check if sensor exists otherwise those two states are indistinguishable.

I don't know if we should change it (read: magic legacy) but then tele.getSensorValue() with proper implementation may solve problem

this refers #777 and #778

[CapnBry]

Yeah it would be great if it returned nil for values that don't exist.

It also would be sweet if getValue() returned two values, the value and true if the sensor value is valid. It is horribly annoying for the value returned to be 0 when telemetry_streaming is false, leading to the Lua having to cache the last value if it wants to keep showing things like the last GPS position or link information like LQ / SNR / etc.

local lq, lq_valid = getValue("RQly")
lcd.drawText(0, 0, "LQ: " .. tostring(lq) .. "%", lq_valid and 0 or INVERS)

-- instead of
last_lq = 0 -- global cache of last value

local lq = getValue("RQly")
if lq == 0 then
  lcd.drawText(0, 0, "LQ: " .. tostring(last_lq) .. "%", INVERS)
else
  lcd.drawText(0, 0, "LQ: " .. tostring(lq) .. "%", 0)
  last_lq = lq
end

[JimB40]

Agree @CapnBry .

Choosing to check if source exists one of valid values (0) is evident bug. Regarding telemetry sources we have three states

1. Source like 'RQly', 'RSSI' are not present (user didn't discover telemetry sensors)
2. Source like 'RQly', 'RSSI' are present but Telemetry stream is not present
3. Source like 'RQly', 'RSSI' are present & Telemetry stream is present

For case 1 it shoud definetely return 'nil'

• function is used for different sources like 'ch.., 'ls..', 'input..' etc
• that will also solve calling wrong name like getValue('RQlz') or getValue('ch88')
• this will create "legacy buzz' as existing scripts expect 0 but sill IMHO we should fix it

For case 2 & 3 to detect if Telemerty stream is available you can use new in EdgeTX LUA function getSwitchValue()

local tele_switch_ID  = getSwitchIndex("TELE")
local tele_ON = getSwitchValue(tele_switch_ID)

• question here is what value telemetry source holds if telemetry stream is lost? Reset to 0 or last passed value

@jfrickmann what you say?

[CapnBry]

Totally agree on case 1. This is breaking behavior but golly it is hard to know what you're working with when "0" is returned for so many cases.

I'm not a fan of having to carry around the "TELE" value anywhere I need to know if the value is valid, and that only works at the TELEMETRY_STREAMING level, not the "Sensor lost" level. Having to cache another ID and passing the tele_ON value to all your rendering functions is an extra step that just isn't needed.

Lua is all about multiple return values. Just return if the sensor value is current when calling getValue(). The developer can then choose between behaviors using the TELE high-level value or the per-sensor local value, would_not_be_in_brackets_in_tlm_view = getValue(X). The getValue implementation has this information available so it is a two line change (one lua push and change the return value to 2) that offers more functionality and convenience to the Lua developer.

[JimB40]

I'm not a fan of having to carry around the "TELE" value anywhere I need to know if the value is valid, and that only works at the TELEMETRY_STREAMING level, not the "Sensor lost" level. Having to cache another ID and passing the tele_ON value to all your rendering functions is an extra step that just isn't needed.

Agree. Forgot that telemetry stream might be available but particular sensor might be lost. So seems your proposal is better.

btw last elrs fw 3.0.1 doesn’t sent RSNR (always zero) and i had quite a long hunt what’s going on :)

[CapnBry]

btw last elrs fw 3.0.1 doesn’t sent RSNR (always zero) and i had quite a long hunt what’s going on :)

It for sure does send RSNR (we rely on this for dynamic power), but it is always 0 in FLRC mode because there is no SNR available in that mode (so dynamic power falls back to RSSI). To keep this on topic, hit me up on our discord if you are experiencing something different.

[jfrickmann]

Actually, @lshems has been complaining about this issue for years 😄

I agree that the most logical API would return nil if there is no value for whatever reason. But I also agree that we should not break the existing API because I know from personal experience how much grief that can get you...

So I support adding a boolean return value to indicate if the value is present. But it is not quite as simple as two lines of code, because getValue(...) can return a:

• Table with GPS coordinates
• Table with date/time
• String of text for sensors with UNIT_TEXT
• Table with battery cell voltages
• A number from a sensor

But in every case, it only returns one argument (a table is just one argument) so adding a second argument as a valid boolean value for the various cases should not be too hard.

If you all agree, then I will get it done.

[CapnBry]

Regardless of the type of the value, the second return parameter is agnostic of the parameter type, being simply: (src < MIXSRC_FIRST_TELEM || src > MIXSRC_LAST_TELEM) || (TELEMETRY_STREAMING() && telemetryItems[X].isAvailable())

However I agree that simplifying it to one line is more confusing than it is worth. 😀

I don't think it should keep returning 0 though when the sensor or telemetry is lost. This leads to having to cache every last value to be able to keep displaying it. The second return value declaring if the "0" is a valid value or not is not nearly as helpful. If getValue() won't be changed for compatibility reasons, I'd suggest a new method be added which returns the current value regardless of TLM_STREAMING && isAvailable. That "0" is utterly useless for the user view, so developers have to fall back on copying the data to keep the display running, and adds more code and storage to the script for every item that needs to be displayed (see my example above). The same EdgeTX code can be used, just that luaGetValueAndPush would need to take a parameter to know if it needs to force pushing a 0 or the actual usable value in case the item isn't current.

[lshems]

Just my point of view:

A measurement is what it is, a measurement. You should always transfer the measurements to your processing system, and let the processing system do what it needs. That way, you always have the maximum available info for the processing system. Knowing there is no measurement is also info.

So now decide if you want a function to provide the measurements and an other to provide processed measurements.

The current is providing processed values.

Add a new function to provide raw measurements from sensors, best even without scaling etc, which should be provided as info as well, to be used in the processing system.

Comparable to the canon RAW picture format and the standard processed jpeg format we all know.

[jfrickmann]

@lshems , I have a Sony camera where RAW files are lossy compressed 🤦 But we could add a getSensorValue function that simply throws whatever it is back in a table including boolean values for streaming and isAvailable. This would not have to deal with all of the other cases for switches, knobs, Tx voltage etc. like getValue.

@CapnBry , the basic getValue function being called from luaGetValueAndPush will return a zero if it does not find the source. It is implemented in mixer.cpp. So when you get a zero in the first place, it can either be that the source actually returns a zero (e.g. 3-pos switch is centered) or it doesn't know the source. Then we have to test if it is UNIT_CELLS, and if the count is zero, because here luaPushCells will push a zero. So there are a few more layers to it.

I do not want to be the dev who changes something like this, and hence becomes the person who has to deal with all of the grief and complaining that follows - just sayin' 😉

[pfeerick]

Path of least resistance - new function that adds the proposed new capability, and depreciate this to be removed in the next major version. When that will be is anyones guess, so there's plenty of "we gave you plenty of time to read the tea leaves" time ;)

Anything else (i.e. changing existing established behaviour), well, the person who wants that is the person who all the complaints from users will be sent to, and will be required to respond to them - "delete" will not be an option! 😝

My 2c on functionality based on what has been said (as I've not look at the code) is it should return nil if not present, last value if not "online", and there should be a flag to say whether it's cached or live. Which fits in perfectly with the getTeleValue() name...

[CapnBry]

Ah see that makes more sense now about how everything returns 0 currently for things that don't exist. I assumed every ID was valid and only the telemetry sensors IDs had the option of not existing or being offline so yeah I get how it is a larger change. Thanks for the education.

I think @pfeerick 's 2 cents is exactly the functionality I've been wanting for years of writing Lua telem scripts and tools. That provides so much more information to be able to display a message if missing sensors need to be discovered, no need to cache last values, and knowing if the value is certified fresh.

[jfrickmann]

So just to be clear - this discussion is all about sensor values, but getValue returns not only sensor values but also other things such as e.g. sticks, sliders, knobs and switches.

So we will not touch getValue. Instead, we will add a new API function that only returns sensor values, and which will do whatever you can agree on in this thread, and which fits within the existing EdgeTX telemetry framework. A few questions:

• What info should be returned by the function under normal conditions? E.g. value (number, table or nil), isAvailable, isStreaming, and isOld?
• What should be returned if the source is not a sensor (e.g. it is a slider)? isAvailable = false or nil?
• What should be returned if the source is not a valid sensor (e.g. no sensor on that id, but it could be a sensor here)? isAvailable = false?
• What should be returned if telemetry is down? Previous value and isStreaming= false?
• What should the API function be named? getSensorValue?

[JimB40]

@jfrickmann

1. Just FIY. I've refreshed for EdgeTX spreadsheet summarizing switches,sources & key events that shows implementation for different radios https://docs.google.com/spreadsheets/d/11gRpQhv6LNhfq85o46ClZguxVQ8xOlNwmqPvndQvntk/edit?usp=sharing

2. As I remember @raphaelcoeffic stated that adding new function consumes memory resources that is particualrly problem for B&W target

3. There is already LUA function model.getSensor(sensorID) that returns various information about source type that is telemetry sensor. Since name "sensor" is used for "source" that is intended for telemetry values and telemetry sensors vary depending on model I'd consider folowing this path with model.getSensorValue(sensorID)

Thing to solve is that sourceID and sensorID have diferent numbering scheme. If you look at implementation in spreadsheet sources being telemetry sensors are the last ones. They start with different sourceID as every radio has different set of physical inputs. So for X9D first telemetry sensor has sourceID 232 but for X10 or TX16S it will be 251. Then sources are populated in groups of 3 for every sensor (value, max value, min value). BTW I think max allowed sesnsors number is 60 as there are 60 fields for telemetry sensors in Companion.

On the other hand sensorID starts from 0.

• for X9D (first telemetry sensor) sensorID == 0 equals sourceID = 232

• for X9D (second telemetry sensor) sensorID == 1 equals sourceID = 235

• for X10 (first telemetry sensor) sensorID == 0 equals sourceID = 251

• for X10 (second telemetry sensor) sensorID == 1 equals sourceID = 254 etc.

Propsal model.getSensorValue(sensor)

• parameter sensor- valid sensorID or sensorName (name returned by getSensor)
• returns (list) value - nil - sensor doesn't exist - otherwise actual telemetry value (might be number, string or table) status (boolean)
  - true - if value is live - false - if value is old (bracketed, sensor or telemetry stream lost) or if value is nil valueMin (optional) - minimum registered telemetry value (for numbers only otherwise nil) valueMax (optional) - maximim registered telemetry value (for numbers only otherwise nil)

Then we should be able to code like @CapnBry propsed

local lq, lq_valid = model.getSensorValue("RQly")
lcd.drawText(0, 0, "LQ: " ..  (lq and tostring(lq) .. "%" or "not discovered", lq_valid and 0 or INVERS)

or list all telemetry sensors

local sID = 0 
while  model.getSensor(sID) do 
  local sValue, sValid = model.getSensorValue(sID)
  local sName = model.getSensor(sensorID).name
  lcd.drawText(0, 0, sName..": " ..  ( type(sValue) ~= 'table' and tostring(sValue) or 'multi' ), sValid and 0 or INVERS)
  sID = sID + 1
end

valueMin & valueMax are optionail to include in implementation as we can always poll via getValue(sensorName..'-') or getValue(sensorName..'+') but including it we have all sensor data in one pass

What you think?

[CapnBry]

I think the proposed solution is perfect and provides all the information anyone would need about a telem sensor with the least code in the Lua.

Is the sensorId still the ID you get from getFieldInfo('XXX').id, or would the IDs need to be pulled from getSensor(string) instead now?

[jfrickmann]

I like that proposal. Instead of having model.getSensorValue take either a string or sensor index argument, I would prefer to add a function model.getSensorId to get the sensor index from the name, just to encourage script writers to get the sensor index once, and use that instead of the name, for the sake of efficiency.

I can also add an iterator function sensors similar to sources and switches if you want it.

I know that Peter voiced some concern about the available memory in BW radios, but I don't know how close to the limit we are today.

[CapnBry]

😄 No memory problems on B&W for any of my telemetry scripts, but boy that ELRS tools script wrecks the whole place with its monolithic nature.

getFieldInfo currently returns IDs for telem items so I am not sure a new function is needed? I mean it certainly is more correct to have getSensorId and getSensorValue that match, but if getSensorId is just a getFieldInfo that only works for a subset of strings IDs, it seems redundant to have two.

[jfrickmann]

There are two meta-indices in ETX. One is sources, where everything that can produce a value (e.g. sticks and telemetry sensors) is indexed, and the other is switches where everything that can produce a boolean (e.g. switch positions and logical swithes) is indexed. And there are direct indexes e.g. one index for logical switches and another for telemetry sensors. What we will do here, is use the direct index for telemetry sensors instead of the meta-index for sources. As JimB40 explained above, these will be different from each another. For the BW radio memory, I think that the issue is flash memory, as we keep adding code, it will eventually overflow so the FW would no longer be able to fit.

[lshems]

FW that won't fit the flash. Sounds like our dynamic code loading in LUA scripts to work around a similar issue. Ever thought of externalising parts of non critical firmware and load onl up that what is needed when it is needed for to ge noncritical functions?

In essence every user screen could be kept on the card, as only one is visible and needs to be loaded at any time....

[gagarinlg]

We are working on freeing up some flash. When the hardware driver update to the HAL is done we can enable LTO which frees some flash.

[CapnBry]

What we will do here, is use the direct index for telemetry sensors instead of the meta-index for sources. As JimB40 explained above, these will be different from each another.

Gotcha. I was aware that the indexes varied based on the hardware, but didn't know what the getSensorValue would not use the virtual (shifted) indexes. Makes sense then.

[jfrickmann]

We are working on freeing up some flash. When the hardware driver update to the HAL is done we can enable LTO which frees some flash.

You guys are awesome! If I understand it correctly, you will update the hardware driver code in the Hardware Abstraction Layer of the code, and when that is done, you can enable the -FLTO compile option to do Link-Time Optimization, and this will enable the compiler to discard some unused segments from the final image, hence freeing up some flash memory?

In that case, we will just keep adding new features until it is full again 🤣

[gagarinlg]

Exactly. Currently we have two different sets of drivers in the firmware, which makes it impossible to use LTO without a lot of warnings.

[jfrickmann]

FW that won't fit the flash. Sounds like our dynamic code loading in LUA scripts to work around a similar issue. Ever thought of externalising parts of non critical firmware and load onl up that what is needed when it is needed for to ge noncritical functions?

In essence every user screen could be kept on the card, as only one is visible and needs to be loaded at any time....

Yes, I did think of it 😉 But I have neither the skills nor the time to do such a thing! But in principle, it would be cool to almost completely eliminate the GUI, add a Lua API that could do anything you need, and then write a complete GUI in Lua with dynamic loading of screens 🤓

[gagarinlg]

I would like to add another UI "mode' whrere a set of lvgl UI elements is available for Lua and Lua creates the screen from those elements. Touch and scrolling would automatically work and reduce the Lua complexity by a lot

[jfrickmann]

ETX 3.0!

[pfeerick]

I know that Peter voiced some concern about the available memory in BW radios, but I don't know how close to the limit we are today.

IIRC it's both flash and RAM, but we regained some of that back with img compression. It was more a gentle "don't go crazy with adding a dozen new functions"... a couple are fine, and ones like this are dearly needed.

FW that won't fit the flash. Sounds like our dynamic code loading in LUA scripts to work around a similar issue. Ever thought of externalising parts of non critical firmware and load onl up that what is needed when it is needed for to ge noncritical functions?

In essence every user screen could be kept on the card, as only one is visible and needs to be loaded at any time....

It's not quite that simple... and not as reliable IMO at this point in time as it means if there is a SD card issue you are hosed. And we are seeing those still every now and then, especially for the B&W targets. So it should stay in flash until we can get any of those are are still firmware related resolved. Plus it takes time for the user education as to ensuring using decent SD cards.

I think a long term goal is to flesh out the LUA API more though so that it can become a viable UI alternative, but it's a backburner project as there's a lot more foundational stuff that needs to be worked on first.

[lshems]

I love these balanced and open discussions on where to go when. Refreshing.

I think some will know why I post this.

[gagarinlg]

I love these balanced and open discussions on where to go when. Refreshing.

I think some will know why I post this.

Do I want to know?

[lshems]

If you don't you don't need to. It's just a compliment then. :)

[JimB40]

Is the sensorId still the ID you get from getFieldInfo('XXX').id, or would the IDs need to be pulled from getSensor(string) instead now?

@CapnBry Nope, as I wrote getFieldInfo('XXX').id is sourceID that is different for the same sensor (like RSSI) for every radio type. On the other hand sensorID used in model.getSensor() implements index unification for telemerty sensors. It starts with 0 and ends with last discovered sensor. So far to check if user discovered telemetry sensor (and how many) I have hard-coded in LUA (for every radio) sourceID allocated for first telemetry sensor and incerement this value looking for nil to get last one. sensorID unifies it across radio targets.

BTW @jfrickmann I think we should change it to start from 1 as LUA tables have this unique feature to start tables with index 1. So having index 0 makes ipairs use imposible.
model.getSensor() is not yet used widely so it shouldn't be a problem. In Companion first telemetry sensor is marked with index 1.

I like that proposal. Instead of having model.getSensorValue take either a string or sensor index argument, I would prefer to add a function model.getSensorId to get the sensor index from the name, just to encourage script writers to get the sensor index once, and use that instead of the name, for the sake of efficiency.

I know about efiiciency and was thinking about that. I've mimicked getValue() that can take both. Don't know how you code but if my code is not effeciency sentensive I prefer to code like Bryan to maintain better code readability local rcLinkSensor = model.getSensorValue("RSSI") looks better for me than local rcLinkSensor = model.getSensorValue(6) Moreover if user redicover telemetry sensors they may appear in different order. So index 6 is no more pointing RSSI. I will keep names lookup table in LUA anyway :)

Just let's keep naming scheme you already started with sources and switches that is model.getSensorIndex(sensorName)

I can also add an iterator function sensors similar to sources and switches if you want it.

Haven't noticed it. Is there a function that returns table of valid sources or switches?