Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Faraday-APRS Persistent Across Network Failures #271

Merged
merged 28 commits into from
Sep 10, 2017

Conversation

kb1lqc
Copy link
Member

@kb1lqc kb1lqc commented Sep 10, 2017

BASE91 APRS Comment Telemetry

aprs.fi and a few other services support BASE91 telemetry transmission. This allows up to 13 bits of ADC telemetry per channel. This PR pulled in what was #269 so here is the information relevant to that PR. @reillyeon beat @el-iso to the punch on there. I appreciate the help from both!

Changelog

  • Imports aprslib and uses base91 functionality. Please note that this is note a standard BASE91 implementation and is a custom APRS version of base91. Qeue XKCD.
  • Removes use of sendTelemetry function as it is no longer needed
  • Only sends scaling values on first transmission to reduce unnecessary packet transmissions. This should be revisited later but seems good for now.
  • Sends a telemetrySequence number that is unbounded to sendPosition as BASE91 comment telemetry can support up to 16 bits of counting and this should be fine for the forceable future :)
  • sendPosition implements BASE91 Comment Telemetry with ADC0, ADC1, ADC3, ADC6, BOARDTEMP, and GPIO bits per Implement APRS BASE91 Comment Telemetry #266.

Raw packets which end in BASE91 comment telemetry
image

12 bit ADC data looks very nice!
image

Persistent APRS-IS Connection Across Network

This PR is based on #269 as it was branched from that PR repository and aims to improve reliability when connecting to APRS-IS servers across a network. As we've seen, often a few minutes to a few hours after starting APRS logging a network event will cause socket ERRNO 10054 or similar and faraday-aprs could not recover. This was annoying at best and at worst rendered the program not terribly useful.

Mind helping me out @reillyeon, if no time I can see if @kb1lqd has some free time in the near future :)

Changelog

  • Created a sendAPRSPacket function to consolidate sending of data to the socket and exception catching
  • Reconnects after a IOERROR in 2 second loop attempts

Issue Tickets

This PR addresses issues related to the following tickets Faraday-Software #119, Faraday-Software #190, Faraday-Software #199, Faraday-Software #201, Faraday-Software #262. A few of these tickets can likely be completely closed as a result of this testing and eventual PR!

Using aprslib with a simple import of base91 function to help create
position comments that will send highly accurate data (up to 16 bits).
Therefor this will help with using all 12 bits of the ADC on Faraday!.
all channels previously sent with telemetry packet are now encoded into
BASE91 format. They are however not used yet.
Needed it for BASE91 APRS transmissions. Also commented out sendTelemetry.
I had to change up how the telemetry sequence counter worked. Also, we
actually can count higher than 255 with BASE91 telemetry so we will do
that. Also, this has a lot of development code but I will clean it up
later as I let it run for a while.
He pointed out that I missed the width setting in his implementation of
BASE91. It appears to be working! My bad...
Removed a bunch of dev code and added normal comments back in.
using integers encoded into BASE91 format I now also send IO data to
APRS-IS with each position!
Decided to only send them once right now during initial loop through
sending. This will hopefully reduce the number of times I have APRS.fi
tell me I'm abusing the network :)
I changed them for development testing and never changed them back. This
function is currently unused as it is.
Lots of development code in here but I can close down the aprs-is server
(aprsc) and start it back up after indication of an error by faraday-aprs.
The program just keeps on going after server starts back up.
Created a function that could be commong to all packet types. This
prevents me from having to duplicate the same code a bunch in aprs.py
Removed old commented out code while also adding documentation for
sendAPRSPacket.
While still commented out, the labels, parameters, and equations packets
are now send on every 10th loop.
Went through all packets being sent and made them all use the
sendAPRSPacket function. This helps clarify the design as well as provide
fault detection and reconnection.
Fixed some errors in the docstring.
ADC names were wrong and I made them not wrong.
Running tests over the day I observed from a local aprsc server that
network connectivity was lost for a few minutes at the same time that my
APRS telemetry stopped. Investigating by disableing my own network
connection for several faraday-aprs loops I observed the ERRNO 10054,
ERRNO 11004, and ERRNO 9 errors. I found that while I closed the socket
aprsSock in the connection code on an IOError I did not recreate a new
socket for aprsSock which resulted in an indefinitely bad file descriptor
(because it didn't exist) whenever I tried to reconnect. This should be
fixed now as I added the automatic aprsSock creation in.
@kb1lqc kb1lqc added this to the Alpha Software milestone Sep 10, 2017
@kb1lqc kb1lqc self-assigned this Sep 10, 2017
@kb1lqc kb1lqc requested a review from reillyeon September 10, 2017 20:02
faraday/aprs.py Outdated
@@ -103,6 +104,11 @@ def aprs_worker(config, sock):
# Local variable initialization
telemSequence = 0

#dev
sock = ''
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is unnecessary.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed, conn=False now used in the initialization just above to force it to initialize on first loop. Thanks for catching this!

faraday/aprs.py Outdated

# Just send labels, Parameters, and Equations every 10th loop
if telemSequence % 10 == 0:

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Extra blank line.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

faraday/aprs.py Outdated
@@ -103,6 +104,11 @@ def aprs_worker(config, sock):
# Local variable initialization
telemSequence = 0

#dev
sock = ''
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: This statement is unnecessary.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

faraday/aprs.py Outdated
except IOError as e:
logger.error(e)
status = sendAPRSPacket(socket, positionString)
return status
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return sendAPRSPacket(socket, positionString)

Here and below.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All return status lines replaced with obviously better return sendAPRSPacket(). Thanks for observing this unnecessary addition of variables!

faraday/aprs.py Outdated
logger.error(e)
aprssock.close()
aprssock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)

else:
logger.info("Connection successful!")
return aprssock
break
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not this change, but break after return does nothing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

faraday/aprs.py Outdated
@@ -762,7 +786,8 @@ def main():
"""

logger.info('Starting Faraday APRS-IS application')
sock = connectAPRSIS()
#sock = connectAPRSIS()
sock = ''
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why pass sock to aprs_worker at all if it calls connectAPRSIS() itself?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I used to need this but with the current implementation of the socket connections I do not. Removed!

Removed the #dev comment as well as manual setting of sock to a string.
Instead I changed conn=False to force it to always initialize inside the
loop.
Removing unnecessary use of status variable by directly returning output
of sendAPRSPacket.
aprs_worker creates it's own socket connection so it's unnecessary to pass
any reference to a socket file descriptor when starting up the thread!
faraday/aprs.py Outdated

sleep(10) # Try to reconnect every 10 seconds
sleep(2) # Try to reconnect every 10 seconds
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update the comment?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

faraday/aprs.py Outdated

# add telemetry to comments
comment = comment + b91Tlm
altComment = altComment + b91Tlm
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment += b91Tlm
altComment += b91Tlm

a += b is more efficient than a = a + b.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

Per @reillyeon a += b is more efficient than a = a + b. Therefore I
changed it.
@kb1lqc
Copy link
Member Author

kb1lqc commented Sep 10, 2017

@reillyeon if you're going to help pull in the code that is effectively PR #269 then I should bring over the changelong here too. I appreciate the help on these if you are doing both!

@reillyeon
Copy link
Collaborator

PR #269 plus the changes here look good to me.

@kb1lqc kb1lqc merged commit fd03b82 into FaradayRF:master Sep 10, 2017
@kb1lqc
Copy link
Member Author

kb1lqc commented Sep 10, 2017

Learn something new everyday... By pulling in #271 github automatically merged #269 since it was based on the same branch. This is awesome and in retrospect obvious! This means that as we progress through a big task we can commit mini PR's on it and still have one big PR at the end. If one PR that is a combination of a few smaller PR's in merged then we no longer have to worry about those. I like that!

@kb1lqd for awareness.

@kb1lqd
Copy link
Contributor

kb1lqd commented Sep 11, 2017

Whoa thanks all and now I learned something new too, very cool PR trait!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants