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

chores: update to LUA 5.3 #4203

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
Draft

chores: update to LUA 5.3 #4203

wants to merge 12 commits into from

Conversation

raphaelcoeffic
Copy link
Member

@raphaelcoeffic raphaelcoeffic commented Oct 14, 2023

Preview of what is coming for 3.0...

Current issues:

  • On T15 the radio goes into EM on shutdown

@JimB40
Copy link
Collaborator

JimB40 commented Oct 15, 2023

Will it increase memory demand? Something is ringing in my head that 5.3 has 32bit numbers so it takes more mem.

@raphaelcoeffic
Copy link
Member Author

Will it increase memory demand? Something is ringing in my head that 5.3 has 32bit numbers so it takes more mem.

The current LUA version we are using only supports 64Bit floating point. So I expect the memory usage to be reduced.

@pfeerick pfeerick requested a review from JimB40 December 9, 2023 03:33
@pfeerick pfeerick added the needs: rebase A git rebase on top of the latest destination branch version is required label Dec 9, 2023
@pfeerick pfeerick removed the needs: rebase A git rebase on top of the latest destination branch version is required label Dec 9, 2023
@LupusTheCanine
Copy link
Contributor

Hi, what is the status of this?

@gagarinlg
Copy link
Member

This will come with EdgeTX 3.0. we want to have one release where we combine multiple incompatible changes, to make life easier for the lua Devs.

@pfeerick pfeerick added this to the 3.0 milestone Jun 17, 2024
@pfeerick pfeerick modified the milestones: 3.0, 2.11 Aug 18, 2024
@pfeerick pfeerick added the needs: rebase A git rebase on top of the latest destination branch version is required label Aug 18, 2024
@JimB40
Copy link
Collaborator

JimB40 commented Aug 20, 2024

@raphaelcoeffic when you rebase pls ping me so I can make some tests.

@philmoz
Copy link
Collaborator

philmoz commented Aug 21, 2024

Rebased to current main; but having issues with crashes to EM and stack overflow in widgets.

@philmoz philmoz removed the needs: rebase A git rebase on top of the latest destination branch version is required label Oct 1, 2024
@philmoz
Copy link
Collaborator

philmoz commented Oct 1, 2024

I've rebases this to current main and it seems to be working ok on the radios I've tested (B&W and Color).

@raphaelcoeffic - I made one small change to ldump.c and lundump.c so that the .luac files generated by the simulator will work on the radio (tested on MacOS). This involves forcing the size_t size value in the header to 32 bits (commit 22219748).

With this change the .luac files generated in the simulator and on the radio are identical.

@philmoz
Copy link
Collaborator

philmoz commented Oct 1, 2024

Compiled .luac files are smaller with 5.3.
E.G. ELRS Lua script:

  • 5.2 = 17758 bytes
  • 5.3 = 15590 bytes

@JimB40
Copy link
Collaborator

JimB40 commented Oct 6, 2024

How to fix it?
What was changed? Bitmap.* global table became bitmap.*?
Changing Bitmap.open to bitmap.open in lua code doesn't work
Screenshot 2024-10-06 at 13 58 30

@philmoz
Copy link
Collaborator

philmoz commented Oct 6, 2024

How to fix it? What was changed? Bitmap.* global table became bitmap.*? Changing Bitmap.open to bitmap.open in lua code doesn't work Screenshot 2024-10-06 at 13 58 30

Odd. Changing it worked for me on the scripts I tested.

@JimB40
Copy link
Collaborator

JimB40 commented Oct 7, 2024

Found it.
It is io.open('file', 'r') that shuts down Simulator (Companion) abrubtly.

Here is test scipt "test_53ioopen.lua"

local fh

return {
  run = function(e)
    return 0
  end,

  init = function()    
    fh = io.open('/SCRIPTS/TOOLS/test_53ioopen.lua')
  end
}

But you can also put in LUA
local fh = io.open('whatever')
to get same result

@JimB40
Copy link
Collaborator

JimB40 commented Oct 7, 2024

io.* library works now.

I've made some inital tests comparing 2.10 LUA 5.2 and 2.11 LUA 5.3 using TX16S simulator.

  1. compiled *.luac files with LUA 5.3 are indeed smaller in size than those compiled with LUA 5.2
    Difference is 10-25% depending on file.

  2. Run-time memory requrement increased about 8-9% :(

Toolbox compiled with 5.2
screenshot_tx16s_24-10-07_13-45-15

Toolbox compiled with 5.3
screenshot_tx16s_24-10-07_13-43-09

Need to do same test with B&W target but that's not good sign as bigger apps like BF Config script already works woth memory limit.

@JimB40
Copy link
Collaborator

JimB40 commented Oct 7, 2024

Even more increase for run-time memory with BW target, ca 11%

Toolbox compiled with 5.2
screenshot_x7access_24-10-07_14-43-08

Toolbox compiled with 5.3
screenshot_x7_24-10-07_14-40-55

@philmoz
Copy link
Collaborator

philmoz commented Oct 7, 2024

I'm seeing reduced memory usage for running scripts.
E.G. ELRS v3 script on T20V2:

  • 5.2 = 44967 bytes
  • 5.3 = 41796 bytes

I can't do any detailed analysis of the memory usage on your toolbox scripts as I can't see the source code anywhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ✨ New feature or request lua
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants