Skip to content

Conversation

pjsg
Copy link
Member

@pjsg pjsg commented Jul 17, 2020

Fixes #3214, #3215.

  • 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/*.

Changing calls to checknumber and pushnumber to be checkinteger and pushinteger as this is (nearly always) the right thing to do.

@nwf
Copy link
Member

nwf commented Jul 18, 2020

These all look fine.

@TerryE
Copy link
Collaborator

TerryE commented Aug 23, 2020

We've also missed a few other modules:

  • mcp4725.c has 4 × lua_tonumber(L, -1) replaced by (uint16) lua_tointeger(L, -1)
  • rtcfifo.c has 2 × lua_tonumber(L, -1) replaced by (uint32_t) lua_tointeger(L, -1) and 2 ditto but without the coercion.

@TerryE
Copy link
Collaborator

TerryE commented Aug 26, 2020

@pjsg, I've gone through the rest of the updates and done another push. There's a conflict in app/modules/sjson.c but I'd like you to review this anyway so you may as well fix this via the web editor. Thankis Terry

@pjsg
Copy link
Member Author

pjsg commented Aug 26, 2020

I've resolved the conflict in sjson.c -- it builds with both 5.1 and 5.3 versions. But no testing beyond that. I'll merge this into my iot device build and give it a whirl.

@TerryE
Copy link
Collaborator

TerryE commented Aug 27, 2020

Thanks Philip. Like you, all I did was a full Travis clean compile on both versions. I don't have the H/W to test a lot of these, but the changes are mainly number to integer swaps when the C-side variables are ints anyway.

@TerryE
Copy link
Collaborator

TerryE commented Aug 27, 2020

@nwf Nathaniel, perhaps you could do a review of these and merge them into dev. Thanks

@TerryE
Copy link
Collaborator

TerryE commented Aug 28, 2020

Just winding you up 😉. Make the change.

@TerryE TerryE merged commit 606f916 into nodemcu:dev Aug 29, 2020
@TerryE
Copy link
Collaborator

TerryE commented Aug 29, 2020

Phil's last change fixed @nwf 's niggle so I reckon that we are good to go on this.

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