Skip to content

Correct warning in lua attitude module#32734

Merged
IamPete1 merged 1 commit into
ArduPilot:masterfrom
peterbarker:pr/lua-attitude-fix
Apr 13, 2026
Merged

Correct warning in lua attitude module#32734
IamPete1 merged 1 commit into
ArduPilot:masterfrom
peterbarker:pr/lua-attitude-fix

Conversation

@peterbarker

Copy link
Copy Markdown
Contributor

Summary

Fixes "_ hints at private" warnings in lua attitude module

Classification & Testing (check all that apply and add your own)

  • Checked by a human programmer
  • Non-functional change
  • No-binary change
  • Infrastructure change (e.g. unit tests, helper scripts)
  • Automated test(s) verify changes (e.g. unit test, autotest)
  • Tested manually, description below (e.g. SITL)
  • Tested on hardware
  • Logs attached
  • Logs available on request

Description

_ is a hint that a variable is unused. Just eliminate the local copy, it's pointless....

@IamPete1

Copy link
Copy Markdown
Member

I think there are lots of scripts with the same pattern.

@timtuxworth timtuxworth left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for cleaning this up. This was just my laziness when translating from c++ to Lua.

Ideally if you could also update the version at the top of the script.

- MAVLinkAttitude.SCRIPT_VERSION = "4.7.0-009"
+ MAVLinkAttitude.SCRIPT_VERSION = "4.8.0-010"

You've probably seen that this displays at boot time, so you know when flying (and in the logs) which version you are running.

@tpwrules

Copy link
Copy Markdown
Contributor

How are you getting a warning?

@peterbarker

Copy link
Copy Markdown
Contributor Author

How are you getting a warning?

~

pbarker@crun:~/rc/ardupilot(master)$ ./Tools/scripts/run_luacheck.sh | grep -v 'OK'
Checking libraries/AP_Scripting/modules/mavlink_attitude.lua 2 warnings

    libraries/AP_Scripting/modules/mavlink_attitude.lua:37:43: used variable _max_lag_ms with unused hint
    libraries/AP_Scripting/modules/mavlink_attitude.lua:37:56: used variable _convergence_loops with unused hint


Total: 2 warnings / 0 errors in 551 files
pbarker@crun:~/rc/ardupilot(master)$ 

Checking libraries/AP_Scripting/modules/mavlink_attitude.lua 2 warnings

    libraries/AP_Scripting/modules/mavlink_attitude.lua:37:43: used variable _max_lag_ms with unused hint
    libraries/AP_Scripting/modules/mavlink_attitude.lua:37:56: used variable _convergence_loops with unused hint
@peterbarker peterbarker force-pushed the pr/lua-attitude-fix branch from ccff704 to 2cd861e Compare April 11, 2026 23:47
@peterbarker

Copy link
Copy Markdown
Contributor Author

Ideally if you could also update the version at the top of the script.

- MAVLinkAttitude.SCRIPT_VERSION = "4.7.0-009"
+ MAVLinkAttitude.SCRIPT_VERSION = "4.8.0-010"

Done.

You've probably seen that this displays at boot time, so you know when flying (and in the logs) which version you are running.

Nope! :-)

@peterbarker

Copy link
Copy Markdown
Contributor Author

@IamPete1 is there any reason we shouldn't fail CI when these things come up? It should generally be clean by the looks of it

@IamPete1

Copy link
Copy Markdown
Member

@IamPete1 is there any reason we shouldn't fail CI when these things come up? It should generally be clean by the looks of it

It passes in CI https://github.com/ArduPilot/ardupilot/actions/runs/24272752988/job/70880824863

Checking libraries/AP_Scripting/modules/mavlink_attitude.lua OK

More testing is more better, there must be some difference between your setup and what we run in CI.

It passes for me too, I get:

luacheck  --version
Luacheck: 0.25.0
Lua: PUC-Rio Lua 5.1
Argparse: 0.6.0
LuaFileSystem: 1.8.0
LuaLanes: Not found

@cclauss

cclauss commented Apr 12, 2026

Copy link
Copy Markdown
Contributor

@peterbarker

Copy link
Copy Markdown
Contributor Author
luacheck  --version
Luacheck: 0.25.0
Lua: PUC-Rio Lua 5.1
Argparse: 0.6.0
LuaFileSystem: 1.8.0
LuaLanes: Not found
pbarker@crun:~$ luacheck --version
Luacheck: 1.1.2
Lua: PUC-Rio Lua 5.1
Argparse: 0.7.1
LuaFileSystem: 1.8.0
LuaLanes: Not found
pbarker@crun:~$ 

@cclauss

cclauss commented Apr 13, 2026

Copy link
Copy Markdown
Contributor

@peterbarker Do you have a local .luacheckrc file?

@peterbarker

Copy link
Copy Markdown
Contributor Author

@peterbarker Do you have a local .luacheckrc file?

No

@cclauss

cclauss commented Apr 13, 2026

Copy link
Copy Markdown
Contributor

Our executable stand-in for a static .luacheckrc file is:
https://github.com/ArduPilot/ardupilot/blob/master/libraries/AP_Scripting/tests/luacheck.lua

% apt-get install lua-check

Ubuntu luacheck
22.04 LTS v0.25.0
24.04 LTS v1.1.2
26.04 LTS v1.2.0

% docker run -it ubuntu:26.04
# apt update && DEBIAN_FRONTEND=noninteractive apt install --yes lsb-release lua-check && lsb_release -ds && luacheck --version

@cclauss cclauss left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Replicated on luacheck v1.2.0 in:

@peterbarker peterbarker requested a review from timtuxworth April 13, 2026 10:53
@peterbarker

Copy link
Copy Markdown
Contributor Author

@timtuxworth still need your approval I think.

@cclauss

cclauss commented Apr 13, 2026

Copy link
Copy Markdown
Contributor

Can we see these warnings on our local machines but not in CI. It would be helpful to replicate them in automated testing before we merge this. Does container: ardupilot/ardupilot-dev-base:v0.1.3 contain an old luacheck?

runs-on: ubuntu-22.04
container: ardupilot/ardupilot-dev-base:v0.1.3

@timtuxworth timtuxworth left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@IamPete1 IamPete1 merged commit 1d8b63e into ArduPilot:master Apr 13, 2026
109 checks passed
@github-project-automation github-project-automation Bot moved this from ReadyForDevCall to Done in Peter's ArduPilot 4.8 Queue Apr 13, 2026
@peterbarker peterbarker deleted the pr/lua-attitude-fix branch April 14, 2026 02:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants