botletics / SIM7000-LTE-Shield

Botletics SIM7000 LTE CAT-M1/NB-IoT Shield for Arduino
https://www.botletics.com/products/sim7000-shield
GNU General Public License v3.0
483 stars 215 forks source link

Timeout fixes #283

Closed foxblock closed 2 years ago

foxblock commented 2 years ago

Some commands on the SIM7000E have a timeout of 65s or more according to spec. In order to accommodate for these I changed all timeout related variables to be uint32_t instead of uint16_t. (not all commands are necessarily needed, I mostly did it for consistency sake)

foxblock commented 2 years ago

Just wondering: why did you revert this? Was there a problem somewhere? Would love to know some details.

botletics commented 2 years ago

I tried it and it threw errors, not exactly sure why.

foxblock commented 2 years ago

Well, okay... Could you be more specific please? Compile errors or runtime errors? Could I bother you to post an error message maybe? I do not get any errors on my end and I would like to help fix it.

My pull request changes some function definitions. Specifically some uint16_t variables to be uint32_t. If your code relies on them being uint16_t it might throw errors, but I am pretty confident that you can just change your code to use uint32_t instead.

As for why I made the change: uint32_t is needed to allow for timeouts larger than 65535ms, which seems long, but consider that the SIM7000E spec defines a max response time of 85s for AT+CIICR for example.

botletics commented 2 years ago

Well, the main reason is that if you have to wait over 65s for a command to run, there's probably something wrong. I think changing to uint32_t is simply unnecessary, but I appreciate the effort.

foxblock commented 2 years ago

Well have a look at the official docs. 65s simply is the defined max timeout for some of these commands. I would rather have a library adhere to the specification set by the actual engineers of the hardware, than use some random value. But it's your project, so I am not going to press the issue any further. Thanks for taking your time.