TheThingsNetwork / arduino-device-lib

Arduino Library for TTN Devices
MIT License
209 stars 96 forks source link

Timed wait for response after resetting the module #216

Open johanstokking opened 7 years ago

johanstokking commented 7 years ago

To know whether there's actually a working module wired and the serial has begun communicating.

See https://github.com/TheThingsNetwork/arduino-device-lib/pull/210#issuecomment-310416492

jpmeijers commented 7 years ago

A good way would be to call sys get ver at statup, and checking if the response is indeed one of the modules the library supports (RN2483, RN2483A, RN2903, RN2903AS).

I'm not sure where is the best place to put this. It feels like something to do directly after the library class was instantiated. In the constructor is not possible because the Stream object was not begin()-ed yet at that stage. The library also doesn't have a single initialisation function. Some of our examples call status() at the start.

That leaves us to have a timeout on the first call to readLine(), and then setting a static flag disabling further timeouts. This however doesn't seem like an elegant solution.

Any other suggestions?

johanstokking commented 7 years ago

How about in reset() ?

jpmeijers commented 7 years ago

reset() is only called at the beginning of personalize() and join(). If communication with the radio doesn't work, and a user calls showStatus(), the library will lock up.

Communication should be checked at the beginning of all public functions that can be called first. Someone might for example call getHardwareEui() at the beginning.

Maybe we should add a function boolean checkHWVersion(), which will return false if the response from the radio is invalid. Then we should call this at the start of all public functions, returning directly with an appropriate error in case of failure. This does however cause unnecessary overhead.

johanstokking commented 7 years ago

Right, then maybe an init() function is better. That would do both this and this connectivity check.