Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid coefficient of trim value #5656

Open
1 task done
chofchop opened this issue Nov 9, 2024 · 5 comments
Open
1 task done

Invalid coefficient of trim value #5656

chofchop opened this issue Nov 9, 2024 · 5 comments
Labels
bug 🪲 Something isn't working triage Bug report awaiting review / sorting

Comments

@chofchop
Copy link

chofchop commented Nov 9, 2024

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

In the current EdgeTX, the trim value takes values ​​from -1049 to 1049.
screen-2024-11-09-222632

Expected Behavior

The trim value should be -1024~1024.
It has been multiplied by an extra 1.024 somewhere.

1024 x 1.024 = 1048.576 ≒ 1049

Steps To Reproduce

Display the trim value using Lua below.

local valuesToTest = {
  'ste',
  'thr',
  'trim-ste',
  'trim-thr',
  'trim-t3',
  'trim-t4',
  'trim-t5'
}
local run = function(evt, ts)
  lcd.clear()
  for i, valueName in ipairs(valuesToTest) do
    local value = getValue(valueName)
    lcd.drawText(10, (i - 1) * 8, valueName..' : '..tostring(value))
  end
  return (evt == EVT_EXIT_BREAK) and 1 or 0
end
return { run=run }

Version

2.10.5

Transmitter

RadioMaster MT12

Operating System (OS)

No response

OS Version

No response

Anything else?

No response

@chofchop chofchop added bug 🪲 Something isn't working triage Bug report awaiting review / sorting labels Nov 9, 2024
@philmoz
Copy link
Collaborator

philmoz commented Nov 10, 2024

It has been like this for a long time - at least 2.8 probably a lot longer.

While it could be changed, there is a risk of affecting existing scripts.

@chofchop
Copy link
Author

Yes, I know that too, but that excuse is not reasonable.
If this obvious mistake is ignored, the number of affected scripts will continue to increase. I can't understand why they didn't fix it as soon as they noticed it.

@pfeerick
Copy link
Member

  1. This may be the first time it has been noticed, since it is still present, no?

  2. This looks like it is only a minor scaling issue, so the risk of affecting existing setups is minimal and at worst requires a minor adjustment. Better to ascertain the cause and resolve it sooner than later.

@philmoz
Copy link
Collaborator

philmoz commented Nov 10, 2024

The cause is this line in mixer.cpp:

    return calc1000toRESX((int16_t)8 * trim_value);

trim_value range is -128 to 128 (or -512 to 512 with extended trims).
8 * trim_value range is -1024 to 1024.

calc1000toRESX assumes the input value range is -1000 to 1000 and scales it up to -1024 to 1024.

@chofchop
Copy link
Author

I am testing by rewriting line 421 of mixer.cpp as shown below.

return (int16_t)8 * trim_value;

Although I have only been able to confirm part of it, it seems to be working properly on both the OS and the script so far.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🪲 Something isn't working triage Bug report awaiting review / sorting
Projects
None yet
Development

No branches or pull requests

3 participants