Fix: custom script output inconsistency between SD play value, model custom script screen and value widget

[mha1]

Fixes the custom script output inconsistencies between SD play value, model custom script screen and value widget. This still allows the use of Custom Scripts as Mixer Source but also in a general way to calculate arbitrary data that can be used in widgets, voice messages and LUA scripts. Backwards compatibility for mixer scripts is maintained.

Fixes #4379

Summary of changes:

• SF play value now announces script outputs as raw value
• model custom script screen now shows script output raw values

Unchanged:

• script output displayed by value widget already showed raw value
• Mixer script backwards compatibility - script outputs will still be scaled to a 1024 based percentage if custom script outputs are used as mixer source. Example: raw value 746 will be 73% (746/1023*100) mixer input

Test script with calculated raw value 2724

local output = { "val1" }

local function run()
  local val1 = 2724
	
  return val1
end

return { run=run, output=output }

1. SF Play Value announces 2724 for tst/val1

2. Value widget shows 2724

3. Custom Scripts screen showing 2724 for tst/val1

4. tst/val1 feed 266% if used as Mixer Source (2724/1024*100)

[philmoz]

Summary of changes:

• SF play value now announces script outputs as raw value
• model custom script screen now shows script output raw values

IMO this breaks existing behaviour and could cause issues for users who expect the current scaled values to be displayed / played.

[mha1]

Summary of changes:

• SF play value now announces script outputs as raw value
• model custom script screen now shows script output raw values

IMO this breaks existing behaviour and could cause issues for users who expect the current scaled values to be displayed / played.

Then two things need to happen:

• stop calling this "Custom Script" and just call it "Mixer Script" (as it was probably implemented as just taht), because with all the output scaling outside the use as mixer script the "Custom", i.e. generalized use of this type of script is nonsense
• @3djc should stop recommending Mixer Scripts as solution for non-mixer applications. This guy just wanted to calculate a parameter and not use the output as mixer source:

[3djc]

The guy wanted to make calculation and output the result as a telemetry sensor (so output is just ignored). Those script with no UI interaction are just perfect for that, run as constant interval, I don't care much on how they are named

[mha1]

The guy wanted to make calculation and output the result as a telemetry sensor (so output is just ignored). Those script with no UI interaction are just perfect for that, run as constant interval, I don't care much on how they are named

If that was the intention he could have tried a function script as I suggested to him. Function scripts calling the run function in the background function work exactly like custom scripts except (the for his use case not needed) inputs/outputs.

Anyhow, using a custom script's outputs for value widgets, SF play value, and the value display in the custom scripts edit screen is more than confusing and useless.

Assume a calculated custom script output value of 2724. This will always be scaled outside the script's control to 266% mixer input (2724*1000/1024 = 266).

Now for different scenarios of using value widget, SF play value and custom script edit screen (in that order):

1. current 2.10: 2724, 266, 2660 -> inconsistent: 2724 makes sense as calculated value, 266 makes sense a scaled mixer value, 2660 makes no sense at all

2. the original proposal in this PR: 2724, 2724, 2724 -> consistent as this is the calculated script value

3. current status of this PR: 2660, 266, 2660 -> inconsistent, see 4.

4. change everything to 266, 266, 266 -> consistent as this is what is fed to the mixer but shows why custom scripts are only of real use if fed in as mixer source.

Let me know what you think it should be. Scrap this PR and just output useless nonsense? I'd like to see inconsistencies ironed out as inconsistencies are always perceived as non-quality.

Btw: the mother of this discussion is the fact 2.8 and following broke users widget scripts by partially removing background processing (#4279). I'd be glad if the "As always, don't break existing scripts..." paradigm was advocated as strong as in this case.

[3djc]

Function script are stopped when one time script are run, which is why using mixer is more reliable for doing calculations

Anyhow...

[mha1]

Function script are stopped when one time script are run, which is why using mixer is more reliable for doing calculations

Anyhow...

Yeah, anyhow. Any opinion on the inconsistent data presentation?

[mha1]

IMO this breaks existing behaviour and could cause issues for users who expect the current scaled values to be displayed / played.

So you're saying the current scaled values should be displayed/played, aren't you? Currently this is not the case, see the underlying issue and my bullet point 1.

[3djc]

I have no opinion on that besides don't break existing scripts

[philmoz]

IMO this breaks existing behaviour and could cause issues for users who expect the current scaled values to be displayed / played.

So you're saying the current scaled values should be displayed/played, aren't you? Currently this is not the case, see the underlying issue and my bullet point 1.

No - I'm not saying the existing behaviour is right or wrong, only pointing out that you are changing the behaviour which could affect existing setups.

[philmoz]

Btw: the mother of this discussion is the fact 2.8 and following broke users widget scripts by partially removing background processing (#4279).

My understanding is that background functions only stop when the user enters one of the setup menus.

I'm still waiting to hear a valid use case for using the setup menus while flying a model.

[mha1]

No - I'm not saying the existing behaviour is right or wrong, only pointing out that you are changing the behaviour which could affect existing setups.

You are pulling my leg, aren't you? A value widget, an SF play function and an edit screen displaying/announcing three totally different values for the same source is correct? And yes, you are right, this is changing existing behavior (have the same value displayed/announced for the same source regardless of the means to display/announce it). This is changing behavior just like any other bug fix does. Fixing a bug is supposed to change behavior, right?

I'm still waiting to hear a valid use case for using the setup menus while flying a model.

Put the channel monitor up on display, e.g. for tuning and quickly checking gyro gain

[pfeerick]

Also keep in mind semiautonomous craft, or or surface craft. You may want to adjust settings while active or run a companion one time script.

On Sat, 9 Dec 2023, 5:51 am Michael, @.***> wrote:

No - I'm not saying the existing behaviour is right or wrong, only pointing out that you are changing the behaviour which could affect existing setups.

You are pulling my leg, aren't you? A value widget, an SF play function and an edit screen displaying/announcing three totally different values for the same source is correct? And yes, you are right, this is changing existing behavior (have the same value displayed/announced for the same source regardless of the means to display/announce it). This is changing behavior just like any other bug fix does. Fixing a bug is supposed to change behavior, right?

I'm still waiting to hear a valid use case for using the setup menus while flying a model.

Put the channel monitor up on display, e.g. for tuning and quickly checking gyro gain

— Reply to this email directly, view it on GitHub https://github.com/EdgeTX/edgetx/pull/4391#issuecomment-1847762207, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJ66KLQ44M3FVL7U5OKCITYINVTLAVCNFSM6AAAAABAMMCQCCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBXG43DEMRQG4 . You are receiving this because you are subscribed to this thread.Message ID: @.***>

[philmoz]

2. the original proposal in this PR: 2724, 2724, 2724 -> consistent as this is the calculated script value
3. change everything to 266, 266, 266 -> consistent as this is what is fed to the mixer but shows why custom scripts are only of real use if fed in as mixer source.

This is why I suggested adding a 'units' property to each the custom script. The default unit would be '%' so the existing behaviour remains (except the value is be displayed as x%). Changing the unit value for a specific script would select option 2.

This keeps exisiting behaviour (with consistent display); but allows you to configure as needed.

[mha1]

2. the original proposal in this PR: 2724, 2724, 2724 -> consistent as this is the calculated script value
3. change everything to 266, 266, 266 -> consistent as this is what is fed to the mixer but shows why custom scripts are only of real use if fed in as mixer source.

This is why I suggested adding a 'units' property to each the custom script. The default unit would be '%' so the existing behaviour remains (except the value is be displayed as x%). Changing the unit value for a specific script would select option 2.

This keeps existing behaviour (with consistent display); but allows you to configure as needed.

Thanks, I already said this is a great idea. But it's not enough to add a unit property to the script. A custom script can have (I think) up to 6 outputs. You'd have to add the unit property to each of the outputs because one might be used as a mixer source, the other as general purpose sources, e.g. for other widgets.

[gagarinlg]

Or am in favor of cleaning the mess up. Since it is not possible without breaking existing stuff, this is going to be a task for 3.0, or whatever we will be calling the release where Lua compatibility is broken

[mha1]

The default unit would be '%' so the existing behaviour remains (except the value is be displayed as x%).

as long as there is no option to change units as you proposed I updated the PR to consistently output the "default" option x%.

[offer-shmuely]

The custom script was not available until 2.9, so how does it break compatibility? Can we assume that there is still very small install base that used it?

[3djc]

Not sure what you mean, they existed already in otx

[pfeerick]

Custom scripts capability has existed for a long time, it was just disabled by default, but could be enabled in OTX via the 'lua' build option in Companion.

On Sun, 10 Dec 2023, 4:39 am 3djc, @.***> wrote:

Not sure what you mean, they existed already in otx

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

[offer-shmuely]

In otx it was, yes, But not in edgeTx (unless you are a developer that can build your own version)

[pfeerick]

The functionality was still there. There was the odd custom build requested since 2.5, and since 2.8.2 was a standard download via the lang-firmware repo. 2.9 just made it turned on by default in the normal firmware. But this is off topic.

If I understand the initial implementation here, it would have in no way broken any existing scripts - as only the cosmetic interpretation of the data (i.e. SF play track, how value shown in custom scripts page) was changed - and it was changed to make it consistent with the actual value, as shown by the value widget. IMO that is the way it should be in the first instance. As far as "could cause issues for users who expect the current scaled values to be displayed / played" ... this is what the "behavioural changes" section of the release notes is for.

I'm going to say this will not make 2.10, but something is clearly needed (as 2724, 266, 2660 is just nonsense), so will mark it for 2.11.

[mha1]

@pfeerick Peter, thanks for sending the cavalry. Do you want me to change the "cosmetic" part to display/announce the raw values the custom script actually outputs (in my example 2724, 2724, 2724) or what a mixer will process, i.e. the scaled value (266, 266, 266). I think it should be the raw value. My reasoning: the script author should know what he/she intends to to with the raw values and the fact that the raw output values are modified outside(!) of the scripts domain leaves only one choice. Raw values should be displayed/announced and the LUA document should explain that the calculated outputs are scaled to (raw*100/1024) when used as mixer source (I'd be happy to do that). And if there is a chance of re-doing the custom script implementation I'd opt for no scaling outside the script's control. If an author wants to use the script as mixer source he/she should calculate the outputs to the appropriate format.

[TodWulff]

As a user who is trying to bitmap a series of binary switches for transmission to a remote aerial payload by way of an RC channel's value (context is ELRS with appropriate full 10-bit switch mode configured therein), I vote for not having a script's return values mangled outside of the control of the script.

Or, an alternative consideration: implement some means to flag 'retain raw values' in the function's definition such that the FW honors this when desired by the script's author. Having an optional 'function flag' to force raw value retention would satisfy my specific challenges while honoring the paradigm of retaining backwards compatibility. Adding a return_raw operator could be a simple means to an end, I perceive.?.

[mha1]

@pfeerick Dude, it'll soon be Xmas again

[pfeerick]

Ok, M, it's Christmas in July :-P

Looking at this a bit more, and also checking OTX, especially where anything colorlcd is concerned, and my conclusion is that it should basically be

4. change everything to 266, 266, 266 -> consistent as this is what is fed to the mixer but shows why custom scripts are only of real use if fed in as mixer source.

colorlcd showing it as the actual value is most likely a bug introduced in opentx 2.4 thus edgetx colorlcd implementation... it was always supposed to be the mixer value if you go back to opentx and how it was done for all three UIs. So all the way through from the value shown in the custom lua screen, to the mixer, to how it was spoken. The only caveat is it appears to always been shown as PREC1 (i.e. 266.2 if it were 2726) but spoken as 266 (i.e. minus the decimal).

Thus...

colorlcd

bw128 channel monitor and outputs screen doesn't show over-values... so will only show 100 but the mixes monitor will show it correctly Only exception will be the telemetry screen, but if it is done there, there won't be enough space as double-sided numbers are being used.

bw212

[!NOTE]
For B&W UI, the value shown in the mixer, is determined by the PPM units value in radio setup - thus 0.0 will be 266.0, and 0.-- (the default) will be 266... and then there is us ;)

[mha1]

Thanks, P, what a nice and well considered late Xmas present. I'm good with 4. It fulfills my requirement, consistency. Looking forward to my upcoming Xmas present - M