GPS Bug 63 + Developer Tutorial Clean by kb1lqd · Pull Request #76 · FaradayRF/Faraday-Software

kb1lqd · 2017-02-17T08:07:43Z

Combining Pull request #68 and a developer tutorial cleanup. Accidentally edited both into a single branch and it's easier to just push them both through.

GPS Bug

Fixed #63 and updated other similar items.

Added length test status and check for global failure to deviceconfig.py so that data too long errors (could update to truncation later if wanted) and displays on deviceconfiguration.py script command prompt
Added other generic length checks for good practice to key fields in device configuration
Added custom leading/trailing padding packet creation functions in commandmodule.py to pad data with specific bytes
Force all Lat, Lon, Alt to match NMEA standards for formatting exept that it allows both leading/trailing padding '0' for Lat/Lon
Lat/Lon/Alt check for incorrect formatting and auto-correct where possible but otherwise error to avoid mis-configuration
Updates disallow negative altitude for now (issues handling the negative) for default programmed GPS configuration location

Device Configuration (General)

Added non-inline INI file comments for format descriptions

#Developer Tutorial Cleaning

Deleted a number of old tutorial files/directories leaving only the intended ..\Faraday-Software\Tutorials\Python_Developer_Tutorials
Fixed basic formatting (minor) and typo's in tutorials 1-Basic_Proxy_Interactions_And_Programming tutorials 0 through 3
- Updated to generic callsign and ID "REPLACEME"

…re_Revision_Telem

Added global failure that prevents programming from occuring if a length is too long.,

Force all GPS lat/lon/alt to float and also create packet with ASCII padding (0x30 for 0's) just like NMEA ASCII uses.

Update fields to generic configurations.

Working com port variable for scripter.

Test.py is programming from command line!

Single BAT file for running working. Not as cleanly modular as I'd like to much better than before.

Finished the example and tutorial first pass.

Fixed absolute links that were broken to relative for GitHub.

Deleted and renamed folders to better organize.

Changed to REPLACEME's local_device_callsign = 'REPLACEME' # Should match the connected Faraday unit as assigned in Proxy configuration local_device_node_id = REPLACEME # Should match the connected Faraday unit as assigned in Proxy configuration

Proxy config

Just inserted a few newlines to clean up output.

…per_Tutorial_Clean

#68 (review) * Prepend instead of append Zero's * Do not convert to float, all Lat, Lon, Alt remains ASCII string * Fixed bug in the submitted deviceconfig.py that padded ALTITUDE with the ALTITUDE_UNIT length (1 instead of 8 bytes)

GGA format.

Removed some engineering data prints.

Working!

As intended!

Cleaned up debug code in device config. Moved inline INI comments to non-inline.

hdkmike · 2017-02-18T04:13:17Z

Applications/Telemetry/telemetry.ini

 units=1
-unit0call=REPLACEME
-unit0id=REPLACEME
+unit0call= REPLACEME


Style: do you have a style reference for your .ini files? Do you want a space between = and REPLACEME

I am open to suggestions @hdkmike , I like the idea of sticking towards what originally seems to be the case of no spaces. Example wikipedia

That's what I thought as well. I vote for sticking with that.

hdkmike · 2017-02-18T04:19:13Z

Faraday_Proxy_Tools/FaradayIO/commandmodule.py

+    padded_data = pad + data
+    return padded_data
+
+


Style: How many newlines do you put between top level defs? For me, it's usually 2.

We're trying to move towards PEP8 or at least very close to it. I believe it's 2 in there as well. We've been scrambling to get functionality and tutorials updated so lots missed in the last few days and in general. Thanks for pointing out! You;re welcome to clone this branch and fix up some styling towards PEP8. That would be super helpful!

hdkmike · 2017-02-18T04:20:26Z

Faraday_Proxy_Tools/FaradayIO/deviceconfig.py

        self.rf_default_boot_freq = [freq_list[2], freq_list[1], freq_list[0]]
        self.rf_PATable = patable_byte
+        return True
+


Style: Method definitions inside a class are surrounded by a single blank line.

hdkmike · 2017-02-18T04:26:40Z

Faraday_Proxy_Tools/FaradayIO/deviceconfig.py

+        altitude_str = commandmodule.create_fixed_length_packet_leading_padding(str(float(altitude_str)),
+                                                                                self.MAX_ALTITUDE_LEN, 0x30)
+
+


Style: Too many blank lines between statements

hdkmike · 2017-02-18T04:27:22Z

Faraday_Proxy_Tools/FaradayIO/deviceconfig.py

        gps = pkt_struct_gps.pack(self.gps_latitude, self.gps_latitude_dir, self.gps_longitude, self.gps_longitude_dir,
                                  self.gps_altitude, self.gps_altitude_units, self.gps_boot_bitmask)

+


Style: too many blank lines between statements

hdkmike · 2017-02-18T04:32:14Z

Applications/Telemetry/telemetry.ini

 units=1
-unit0call=REPLACEME
-unit0id=REPLACEME
+unit0call= REPLACEME


That's what I thought as well. I vote for sticking with that.

kb1lqc · 2017-02-18T05:57:49Z

Checked simply programming a unit with bogus but valid data. Latitude, Longitude and altitude appeared to have updated nominally:

Moving to 9999.99 meters altitude and South/East coordinate direction also appeared nominal:

kb1lqc

Left lots of comments. Overall I agree with @hdkmike that we should improve formatting more towards PEP8 but I think we should let it pass on this commit. Let's get good functionality and then in a new pull request attack code style.

Overall the update appears to work but I did not physically test bootstrap loading. You need to sanitize input better per my comments and possibly some I missed.

I am not sure why you are moving the Tutorials folder?

kb1lqc · 2017-02-18T06:02:22Z

Faraday_Proxy_Tools/FaradayIO/commandmodule.py

+    """
+    pad_len = fixed_legth - len(data)
+    pad = chr(padding_byte) * pad_len
+    pad = pad


Why pad = pad?

Yep, pretty much does what it says.

Removed on all 3 padding functions.

kb1lqc · 2017-02-18T06:04:50Z

Faraday_Proxy_Tools/FaradayIO/commandmodule.py

+    :Example:
+
+        >>> pack_data("Test data message", 20, 0x30)
+        'Test data message\x00\x00\x00'


You're example differs from the example code output. When I chr(0x30) I get "000" which is not "\x00\x00\x00"

I updated to show a better example for all 3 padding functions:

kb1lqc · 2017-02-18T06:08:11Z

Faraday_Proxy_Tools/FaradayIO/commandmodule.py

+    """
+    pad_len = fixed_legth - len(data)
+    pad = chr(padding_byte) * pad_len
+    pad = pad


Again why pad=pad?

Removed as stated above.

kb1lqc · 2017-02-18T06:09:17Z

Faraday_Proxy_Tools/FaradayIO/commandmodule.py

+    :Example:
+
+        >>> pack_data("Test data message", 20, 0x30)
+        '\x00\x00\x00Test data message'


Again your example differs from the input 0x30 padding

See #76 (comment)

Fixed.

kb1lqc · 2017-02-18T06:19:43Z

Faraday_Proxy_Tools/FaradayIO/commandmodule.py

        return padded_data


+def create_fixed_length_packet_padding(data, fixed_legth, padding_byte):


I feel like this could be less brute force by doing something similar to what I did in my aprs.py formatting:

https://github.com/FaradayRF/Faraday-Software/blob/master/Applications/APRS/aprs.py#L137
For example I performed
latDec = str("%.2f" % latDec).zfill(5)

This rounded an input float to two decimal places followed by string conversion. Then ensured the entire string was five characters long, including the decimal point, appending zeros (strings) to the end to fill up.

To do leading Zeros you could split() the string at the decimal point and take the int to the left of the decimal and then use string formatting with something like ("%02d" % int(a)).zfill(5) which would fill up to five characters with leading zeros. Then join*() the strings back together. This uses built in functions in a pretty straightforward way to me.

I see your point for Zfill on the leading zeros but what about trailing? I can use Zfill and remove trailing zero support completely but how would I ensure the size 4 bytes on righthand decimal side?

Trailing zeros is exactly what the "%.2f" above does. Notice the two different formatting statements.

kb1lqc · 2017-02-18T07:20:13Z

Firmware_Bootstrap_Loader/faradaybsl/createtiscripter.py

+import array
+import sys
+
+#filename = sys.argv[1]


Committing a bunch of commented out code?

kb1lqc · 2017-02-18T07:20:26Z

Firmware_Bootstrap_Loader/faradaybsl/createtiscripter.py

+        self.CreateOutputFile()
+        self.CreateBslScript()
+
+    # ParseTiTxtHexFile(file_program_hex)


kb1lqc · 2017-02-18T07:21:03Z

Firmware_Bootstrap_Loader/faradaybsl/createtiscripter.py

+
+    def CreateBslScript(self):
+        #global device_com_port
+        #com_string = 'MODE 6xx UART 9600 COM%d PARITY' % device_com_port


Old string?

kb1lqc · 2017-02-18T07:24:43Z

Tutorials/start/hello-world.md

@@ -55,7 +55,7 @@ To run the code example we've placed hello-world.py in the start folder
 Ensure that Proxy is running in the background!

 ### Linux (Debian-based)
- * Navigate in terminal to `/git/faraday-software/Tutorials/start` and run `sudo python hello-world.py`
+ * Navigate in terminal to `/git/faraday-software/Python_Developer_Tutorials/start` and run `sudo python hello-world.py`


Why are you changing this? I direct faradayrf.com/start to the readme in Tutorials/start, we need to coordinate this.

kb1lqc · 2017-02-18T07:25:09Z

docs/conf.py

@@ -19,7 +19,7 @@
 import os
 import sys
 sys.path.insert(0, os.path.abspath('../Faraday_Proxy_Tools/FaradayIO')) #FaradayIO toolset
-sys.path.insert(0, os.path.abspath('../Tutorials/FaradayIO/Tutorial_0')) #FaradayIO toolset
+sys.path.insert(0, os.path.abspath('../Python_Developer_Tutorials/FaradayIO/Tutorial_0')) #FaradayIO toolset


Again, why changing? Needs coordination.

Fixed padding function examples to have more accurate examples.

For callsign, callsign id, and GPIO defaults.

Min and Max allowable frequency.

kb1lqd · 2017-02-18T08:45:27Z

@kb1lqc This pull request has changed from fixing the noted issue for GPS into a "implement complete configuration file checking"

Updated a few relavent ones but this is just crazy out of scope for this puyll request. Will make future issue enhancement. All intended updates are shown working, optimized or not.

kb1lqd · 2017-02-18T09:22:18Z

#80

kb1lqc

I've made my comments and overall we just need the fixes this offers. We'e created issue tickets for relevant fixes we know need to be made to these files. Time to move on.

kb1lqc · 2017-02-18T18:48:27Z

Faraday_Proxy_Tools/FaradayIO/commandmodule.py


-            >>> pack_data("Test data message", 20)
+            >>> testdata = create_fixed_length_packet("Test data message", 20)
+            >>> print repr(testdata)


Should eventually remove test print cases

kb1lqc · 2017-02-18T18:48:47Z

Faraday_Proxy_Tools/FaradayIO/commandmodule.py

-        >>> pack_data("Test data message", 20, 0x30)
-        'Test data message\x00\x00\x00'
+        >>> testdata = create_fixed_length_packet_padding("Test data message", 20, 0x7E)
+        >>> print testdata


Should eventually remove

kb1lqc · 2017-02-18T18:49:04Z

Faraday_Proxy_Tools/FaradayIO/commandmodule.py

-        >>> pack_data("Test data message", 20, 0x30)
-        '\x00\x00\x00Test data message'
+        >>> testdata = create_fixed_length_packet_leading_padding("Test data message", 20, 0x7E)
+        >>> print testdata


Should eventually remove

kb1lqc

We've made necessary fixes and created relevant issue tickets for more thorough checks and formatting. Let's move on.

Brenton Salmi added 30 commits February 11, 2017 11:20

Merge remote-tracking branch 'refs/remotes/origin/master' into Firmwa…

e69dc49

…re_Revision_Telem

Basic checks updates in device configuration files.

24308fe

Adding temp proxy settings

9d81ffb

Correctly checking and erroring on config to long lengths

abf2108

Added global failure that prevents programming from occuring if a length is too long.,

Completed initial effort

14c90df

Force all GPS lat/lon/alt to float and also create packet with ASCII padding (0x30 for 0's) just like NMEA ASCII uses.

Removed prints and added comments.

4494a8c

Generic configuration

59d21bf

Update fields to generic configurations.

Generic.

ad95cde

New scripts creating memory checks

d70fb58

Cleaned version creating TI script

ae146da

Com port variable

b8b1a72

Working com port variable for scripter.

Added cmd ARGUMENTS

0b85944

Test programming

0d781ae

Test.py is programming from command line!

Deleted Old Files

758a174

Cleaned a bit

9ff1681

Firmware Folder

a04c949

Cleaned up single BAT

542ff0e

Single BAT file for running working. Not as cleanly modular as I'd like to much better than before.

Finalized

3d3642e

Finished the example and tutorial first pass.

Fixed README images

c4447a0

Fixed absolute links that were broken to relative for GitHub.

Basic folder structure cleaning

0ff1b1a

Deleted and renamed folders to better organize.

Generic proxy settins tutorial

a810f1c

Changed to REPLACEME's local_device_callsign = 'REPLACEME' # Should match the connected Faraday unit as assigned in Proxy configuration local_device_node_id = REPLACEME # Should match the connected Faraday unit as assigned in Proxy configuration

Fixed Tutorial 0

13e1364

Tutorial 0 Generic

d5253c6

Proxy config

Cleaned Tutorial 2

1c06425

Just inserted a few newlines to clean up output.

Merge remote-tracking branch 'refs/remotes/origin/master' into Develo…

f2b4ef8

…per_Tutorial_Clean

Fixed Bryces comments

704faf6

#68 (review) * Prepend instead of append Zero's * Do not convert to float, all Lat, Lon, Alt remains ASCII string * Fixed bug in the submitted deviceconfig.py that padded ALTITUDE with the ALTITUDE_UNIT length (1 instead of 8 bytes)

Updating with formatting check

31b3d0f

Format GPS default flash NMEA

bbcfee3

GGA format.

Cleaned

5cc7fa3

Removed some engineering data prints.

GPS checking format working

6152ef9

Working!

Brenton Salmi added 4 commits February 15, 2017 00:42

All checks for GPS formatting work

397f717

As intended!

Replaced with generic configuration

7585f68

Generic config

200f79b

Cleaned up code

334c14e

Cleaned up debug code in device config. Moved inline INI comments to non-inline.

kb1lqd mentioned this pull request Feb 17, 2017

Patched Bug #63 #68

Closed

kb1lqd requested a review from kb1lqc February 17, 2017 09:12

kb1lqd assigned kb1lqc and kb1lqd Feb 17, 2017

kb1lqd added bug Cleaning labels Feb 17, 2017

kb1lqd added this to the Initial Code Release milestone Feb 17, 2017

hdkmike reviewed Feb 18, 2017

View reviewed changes

kb1lqc previously requested changes Feb 18, 2017

View reviewed changes

Brenton Salmi added 4 commits February 17, 2017 23:46

Fixed padding function examples

3d13fea

Fixed padding function examples to have more accurate examples.

Removed stray pad = pad

c1d8087

Added basic info check and errors

d573b94

For callsign, callsign id, and GPIO defaults.

Adding check for frequency

51fe932

Min and Max allowable frequency.

kb1lqd mentioned this pull request Feb 18, 2017

Make Configuration INI checker And Use Logging #80

Open

kb1lqc approved these changes Feb 18, 2017

View reviewed changes

kb1lqd merged commit 8e9b19c into master Feb 18, 2017

kb1lqc deleted the Developer_Tutorial_Clean_Lastcommit branch March 4, 2017 21:25

		altitude_str = commandmodule.create_fixed_length_packet_leading_padding(str(float(altitude_str)),
		self.MAX_ALTITUDE_LEN, 0x30)

		gps = pkt_struct_gps.pack(self.gps_latitude, self.gps_latitude_dir, self.gps_longitude, self.gps_longitude_dir,
		self.gps_altitude, self.gps_altitude_units, self.gps_boot_bitmask)

		return padded_data


		def create_fixed_length_packet_padding(data, fixed_legth, padding_byte):

Conversation

kb1lqd commented Feb 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

GPS Bug

Device Configuration (General)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kb1lqc commented Feb 18, 2017

Uh oh!

kb1lqc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kb1lqd commented Feb 18, 2017

Uh oh!

kb1lqd commented Feb 18, 2017

Uh oh!

kb1lqc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kb1lqc left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

kb1lqd commented Feb 17, 2017 •

edited

Loading