nodemcu / nodemcu-firmware

Lua based interactive firmware for ESP8266, ESP8285 and ESP32
https://nodemcu.readthedocs.io
MIT License
7.64k stars 3.12k forks source link

rtctime.dsleep fails with integer argument on Lua 5.3 build #3453

Open lsborg opened 3 years ago

lsborg commented 3 years ago

Expected behavior

rtctime.dsleep(2808000000) should put microcontroller to sleep

Actual behavior

Lua error: stdin:1: bad argument #1 to 'dsleep' (number has no integer representation) stack traceback: [C]: in function 'rtctime.dsleep' stdin:1: in main chunk [C]: in ? [C]: in ?

Test code

Provide a Minimal, Complete, and Verifiable example which will reproduce the problem.

rtctime.dsleep(2808000000)

NodeMCU startup banner

NodeMCU 3.0.0.0 built with Docker provided by frightanic.com branch: release commit: d4ae3c364bd8ae3ded8b77d35745b7f07879f5f9 release: 3.0.0-release_20210201 +1 release DTS: 202105102018 SSL: false build type: float LFS: 0x10000 bytes total capacity modules: adc,encoder,file,gpio,http,net,node,rtctime,sjson,sntp,tmr,uart,wifi build 2021-08-01 02:08 powered by Lua 5.3.5 on SDK 3.0.1-dev(fce080e)

Hardware

Nodemcu ESP8266

Other

node.dsleep(2808000000) works fine.

marcelstoer commented 3 years ago

rtctime.dsleep() uses integer internally: https://github.com/nodemcu/nodemcu-firmware/blob/dev/app/modules/rtctime.c#L188. Hence, the maximum is 4294967295 as documented: https://nodemcu.readthedocs.io/en/latest/modules/rtctime/#rtctimedsleep. node.dsleep() on the other hand uses long (64bit) internally: https://github.com/nodemcu/nodemcu-firmware/blob/dev/app/modules/node.c#L171. See the note about the maximum in the docs: https://nodemcu.readthedocs.io/en/latest/modules/node/#nodedsleep

However, as your 2808000000 value is below the 4294967295 maximum I don't understand what's going on here.

lsborg commented 3 years ago

Thanks for your answer @marcelstoer

New info, it works with lua 5.1

NodeMCU startup banner

NodeMCU 3.0.0.0 built with Docker provided by frightanic.com branch: release commit: d4ae3c364bd8ae3ded8b77d35745b7f07879f5f9 release: 3.0.0-release_20210201 +1 release DTS: 202105102018 SSL: false build type: float LFS: 0x10000 bytes total capacity modules: adc,encoder,file,gpio,http,net,node,rtctime,sjson,sntp,tmr,uart,wifi build 2021-08-01 23:28 powered by Lua 5.1.4 on SDK 3.0.1-dev(fce080e)

marcelstoer commented 3 years ago

@jmattsson is there a good reason rtctime.dsleep should not be modeled after what we have in node.dsleep (using uint64 us;)?

https://github.com/nodemcu/nodemcu-firmware/blob/949875d590e771603a73ed3a90928d48ea3dd38b/app/modules/node.c#L182-L189

jmattsson commented 3 years ago

IIRC (and it's been years since I worked in this area), the sleep time limitation came from the 32bit registers used to configure the chip for wakeup.

marcelstoer commented 3 years ago

Oh, true, it even says so in the rtcmem docs. What could be the reason 2808000000 works with Lua 5.1 but not with Lua 5.3?

jmattsson commented 3 years ago

Based on that error message, it looks to me like either lua_tounsigned() is still trying to fit it into a signed 32bit integer (max value 2147483648), or the lua_isinteger() is the one doing it. Is there a lua_isunsigned() in Lua 5.3?