adammck/rubygsm

Infinite ack (+CNMA) and reboot (+CFUN) loop.

Opened this issue · 3 comments

liehann commented

Let me start by saying thanks for making this library available. Secondly I realise that you haven't touched the code in a long time, so I'm not expecting you to look into this, but maybe you have an idea of how to fix it that would be consistent.

I ran into an issues around retries. I received a message which the library then attempts to ack with a +CNMA.
The modem responds with +CMS ERROR 340, which is "No +CNMA acknowledgement expected". The command is retried 5 times, all of which fail. The device is then reset with a +CFUN command, which succeeds, and the command is retried. The code then gets stuck in an infinite loop of try and fail command, reset device.

This problem will always be triggered with a command that fails legitimately (ie. it will always fail), and a modem that always resets successfully.

I see a few options:

  1. A simple workaround of making @reset_on_failure false. Based on your comments it does seem like the reset is a very useful feature, so I don't really want to change that.
  2. Change parse_incoming_sms! to use try_command for +CNMA. Any errors rising from that command are ignored anyway (they are just logged). I like this, but it still doesn't solve the root problem.
  3. Only reboot once in command. eg. Try 5 times, reboot, try 5 times again, propgate error.

I like the idea of doing both 2 and 3, so I'll throw together a patch, unless you have a better idea.

Thanks,
Liehann

liehann commented

I've added two commits to my branch. I'm not sure how to do a pull request with only those commits, and I've changed a bunch of other stuff too - so feel free to take a look if you like, or not.

adammck commented

i don't use rubygsm for much these days, but i'm glad you're finding it useful.

this should indeed be more robust; i've seen more than one modem reject +CNMA. your fix looks good. if you'd be so kind as to extract the relevant commits into a branch and create a pull request, i'll merge them straight away. otherwise, i'll try to get them merged some time this week.

thanks!

liehann commented

I found a nasty race condition with the locking stuff. I've replaced the custom locks with a Monitor which should resolve the issue. I'm under some pressure to get my system working, but as soon as I have the time I'll see if I can make a branch with all relevant commits. Otherwise, if you're not opposed, I'll just make a pull request for everything and you can feel free to commit when and if you want.