search

adafruit / Adafruit_CircuitPython_RockBlock

CircuitPython driver for Rock Seven RockBLOCK Iridium satellite modem
MIT License
4 stars 10 forks source link

Add features: IMEI, RSSI, Versions, Geolocation, Time, Ring Alerts, handle non-responsive modem #15

Closed zunkworks closed 3 years ago

zunkworks commented 3 years ago

I added functionality to return the modem's IMEI (serial number), RSSI (signal strength), firmware versions, it's geolocation per the Iridium network, the date/time per Iridium network, the ability to enable/disable ring alerts, and lastly the ability to check for ring alerts.

I made changes to handle a non-responsive modem (in case it is powered off) by modifying _uart_xfer to return None if there is no response from the modem. This will require additional changes to several other functions. I'm not sure this was the best way to do it, so I have not updated all the other functions yet. I would like to get some feedback first. If you have a better suggestion, or think this is ok, please let me know and I'll update accordingly.

Thank you, Jeremy

zunkworks commented 3 years ago

Phew, now passes CI checks :)

caternuson commented 3 years ago

Thanks for these additional updates. I've added a lot of commentary, but please realize they are mostly cosmetic with respect to all the various new properties you've added.

Was there a need for the changes to _uart_xfer? That's the main shared comm function used for the library, so changes to that are a little more scrutinized. EDIT - Oh, just saw you talk about this a bit in your original post. I'd say revert the _uart_xfer back to the way it was for now. This PR can just focus on the adding of the new properties. And then can open a new issue for dealing with unresponsive modem and discuss there what makes sense.

zunkworks commented 3 years ago

Hi Carter, thank you very much for the feedback. I'll start working on the revisions you suggested in your comments. I'll get back to you when I have my code ready for another review or have questions. Thanks again! - Jeremy

zunkworks commented 3 years ago

Thanks, I've changed the code per your comments and pushed them.

I have been testing on a 9603 using a Feather M4 Express prior to each commit. The modem revision I've mostly been testing on: ('Call DSP Version: 1.7 svn: 2358', 'DBB Version: 0x0001 (ASIC)', 'RFA Version: 0x0007 (SRFA2)', 'NVM Version: KVS', 'Hardware Version: BOOT07d2/9603NrevDE/04/RAW0c', 'BOOT Version: TA16005 (rev exported)') Do you think I should update an example or write a new example to include the new functionality?

Speaking of testing, on the next PR I do I'd like to add +GEMON which returns an estimate of the microamp hours the modem has consumed. I think it would be very useful for anyone running off batteries/solar/etc. However the software revision I have on several modems does not support this. I do have an older modem that does support it, and RockBlock support shows it working on a newer revision than what I have. It could be added as long as it can handle the return code of ERROR. I've written this code, but didn't push it since it wasn't working on all of my modems.

And speaking of errors and future PRs, if the modem is powered down, then the init fails because it calls reset: Traceback (most recent call last): File "code.py", line 22, in File "./adafruit_rockblock.py", line 60, in init File "./adafruit_rockblock.py", line 80, in reset File "./adafruit_rockblock.py", line 72, in _uart_xfer File "./adafruit_rockblock.py", line 70, in TypeError: 'NoneType' object is not iterable

Which is because line is equal to None, and the while loop can't iterate over None. while not any(EOM in line for EOM in (b"OK\r\n", b"ERROR\r\n")): line = self._uart.readline() resp.append(line)

That's why I was changing reset and _uart_xfer. But we'll save that for another PR as you suggested.