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

Add support for using doubles in the LUA53 build. #3225

Merged
merged 9 commits into from
Nov 7, 2020

Conversation

pjsg
Copy link
Member

@pjsg pjsg commented Jul 18, 2020

Fixes #3224.

  • This PR is for the dev branch rather than for master.
  • This PR is compliant with the other contributing guidelines as well (if not, please describe why).
  • I have thoroughly tested my contribution.
  • The code changes are reflected in the documentation at docs/*.

This brings back support for using doubles as the floating point format in the LUA53 build. This makes it much more compatible with the old LUA51 firmware.

Copy link
Member

@HHHartmann HHHartmann left a comment

Choose a reason for hiding this comment

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

Some minor nitpicks but in general I think this is a good idea

app/include/user_config.h Outdated Show resolved Hide resolved
app/include/user_config.h Outdated Show resolved Hide resolved
app/include/user_config.h Outdated Show resolved Hide resolved
app/lua53/luaconf.h Outdated Show resolved Hide resolved
docs/build.md Outdated Show resolved Hide resolved
docs/compiling.md Outdated Show resolved Hide resolved
docs/build.md Outdated Show resolved Hide resolved
docs/build.md Outdated Show resolved Hide resolved
app/lua/luaconf.h Outdated Show resolved Hide resolved
docs/build.md Outdated Show resolved Hide resolved
@TerryE
Copy link
Collaborator

TerryE commented Jul 19, 2020

See comments on #3224. The Lua VM evaluates integer expressions using the default (sint32) type. Adding doubles alone will work in a very counter intuitive way for developer familiar with the 5.1 VM behaviour.

@pjsg
Copy link
Member Author

pjsg commented Jul 19, 2020

This version changes the option for a build to be double & 64-bit ints. In retrospect, I should have done this first time around.

@pjsg pjsg requested a review from HHHartmann October 4, 2020 21:50
@pjsg
Copy link
Member Author

pjsg commented Oct 5, 2020

@HHHartmann I think that I have addressed all your issues.

@marcelstoer
Copy link
Member

@nwf @pjsg as discussed, ok for you if we land this immediately after the next master drop (~1 week from now)?

@pjsg
Copy link
Member Author

pjsg commented Oct 25, 2020 via email

@marcelstoer marcelstoer modified the milestone: Next release Nov 3, 2020
@marcelstoer marcelstoer merged commit f67792e into nodemcu:dev Nov 7, 2020
vsky279 pushed a commit to vsky279/nodemcu-firmware that referenced this pull request Nov 21, 2020
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.

4 participants