Closed kb1lqc closed 7 years ago
Oh cool. So here are my thoughts:
Input validation. I've just pushed changes to PositionReport
that validate latitude
and longitude
. See https://github.com/rossengeorgiev/aprs-python/commit/7d5dafa649625b9d0d1628514fb9fa2e273c1a04
I don't like each value being a separate parameter, instead of a list
. Currently the module will parse comment embedded telemetry and output a list
. Besides consistency, it will simplify input validation
OK @rossengeorgiev I updated per your comments:
I did not yet perform any validation. I understand attribute setter with a single variable but don't know how to get it to work with a dictionary of lists. Thoughts?
What do you think about validation @rossengeorgiev?
I see why you did them that way, but comment telemetry is an addon to position reports. This is a telemetry report, and should have the values as properties like before. I would still keep the vals as list
and modify/validate them through a setter function. Their names don't need to necessary line up with the names from the dict
. If you want I can give it a go?
@rossengeorgiev if you want feel free to give it a go from my work if you'd like. Glad to help where I can.
Modeled off of the PositionReport class, I created a TelemetryReport and used similar serialization to create a working packet that was uploaded to aprs.fi for testing.
This is my first iteration of working code that I confirmed would be accepted:
The string above has a blank comment at the end.
This code also does no input checking. One could break it by entering analog values > 255 or strings for their values. A larger or smaller array of digital values entered would also cause some issues.
I'd like to get your take on the code and I'd preferably ad some documentation and a Docstring unless you think otherwise. Thoughts?