Add gps.py part #992

Ezward · 2022-03-06T21:49:17Z

Part of Issue #991

reads gps coordinates as tuple (timestamp, longitude, latitude)
can be run in a thread
main self test logs gps coordinates
self test will also capture waypoints, model them statistically, plot them and do several kinds of hit tests against them. This really good for getting your gps setup well.

Ezward · 2022-03-07T07:25:12Z

Note the changes to install; I'm having trouble testing them because of the conda issue with solving the environment.

DocGarbanzo

In general this looks great, very good and clear/readible code. I only have a couple of minor comments.

DocGarbanzo · 2022-03-14T08:42:54Z

donkeycar/parts/gps.py

+import utm
+
+logger = logging.getLogger("gps")
+logger.setLevel(logging.DEBUG)


I think we should set the logging level only in manage.py and then all parts will log according to that level.

Yes, left over debugging code. Thanks

DocGarbanzo · 2022-03-14T08:46:18Z

donkeycar/parts/gps.py

+                line = self._readline()
+                if line:
+                    if self.debug:
+                        logger.info(line)


Here you would say logger.debug(...)

I actually want this to log at info level, but only if debugging is turned on. I don't want to turn on all debug level logs.

I think, that having the functionality of the logging module which allows a very fine grained adjustment of which messages in which modules will perform logging is contradictory to having an additional boolean debug flag here. So you would make this a debug statement here.

Logging module won't do what I want unless I can set the level per logger, which itself is a pain; we could setup a logging configuration file to make this happen, but this is simple and does what I need. See more comments below.

donkeycar/parts/gps.py

DocGarbanzo · 2022-03-14T19:36:31Z

donkeycar/parts/gps.py

+        if self.gps is not None:
+            with self.lock:
+                try:
+                    if self.gps is not None and self.gps.is_open:


self.gps has already been checked.

Thanks for catching this.

DocGarbanzo · 2022-03-14T19:37:01Z

donkeycar/parts/gps.py

+            if self.lock.acquire(blocking=False):
+                try:
+                    # TODO: Serial.in_waiting _always_ returns 0 in Macintosh
+                    if self.gps is not None and self.gps.is_open and self.gps.in_waiting:


self.gps has been checked already

DocGarbanzo · 2022-03-14T19:50:45Z

donkeycar/parts/gps.py

+        """
+        Calculate (min, max, mean, std_deviation) of a list of floats
+        """
+        if data is None or len(data) == 0:


If not data:

donkeycar/parts/gps.py

DocGarbanzo · 2022-03-14T20:01:22Z

install/envs/mac.yml

    - moviepy
+    - install -U --no-deps numpy==1.19.4


I think this does not work.

fixed this to make it a legal set of arguments for pip install

DocGarbanzo · 2022-03-14T20:02:55Z

install/envs/mac.yml

  - psutil
  - kivy=2.0.0
  - plotly
  - pyyaml
  - fastai
  - pip:
-    - tensorflow==2.2.0
+    - --no-binary :h5py: h5py==3.1.0
+    - tensorflow==2.5.3


Could it be these changes crept in accidentally, they don't look right....

I fixed this so it is legal arguments to pip install. These have been tested using pip directly. However, I can't test this using the standard install instructions because conda never resolves the environment; after 12 hours I quit.

I cannot get the conda install instructions to work at all. I got a pip install script to work on mac, but I did have to install h5py this way so it would build and tensorflow 2.2.0 is not longer available. I think we need to do some basic testing on the install in all the environments; I think the RPi is broken because of tensorflow 2.2.0 as well.

I would be happy to back out all the setup changes and we can deal with them in a separate pull request to fix the install issues on every platform.

Yes, please revert those change, the Mac conda environment runs well on the CI as well as on my computer. If the RPi install gives a problem then we need to address this differently, as the RPi installs through pip (using setup.py) only.

I reverted mac.yml to be same as main.

DocGarbanzo · 2022-03-20T19:33:25Z

donkeycar/parts/gps.py

+
+logger = logging.getLogger("gps")
+
+class gps:


Missed that: class names should be upper/camel case.

I made it Gps as GPS looks like a constant.

DocGarbanzo · 2022-03-20T19:42:59Z

donkeycar/parts/gps.py

+import serial
+import utm
+
+logger = logging.getLogger("gps")


If you don't use __name__ as the name of the logger, it won't inherit the settings from the base logger (actually any base logger in the hierarchy).

I fixed that. Thanks.

donkeycar/parts/gps.py

DocGarbanzo · 2022-03-20T19:43:49Z

donkeycar/parts/gps.py

+                line = self._readline()
+                if line:
+                    if self.debug:
+                        logger.info(line)


I think, that having the functionality of the logging module which allows a very fine grained adjustment of which messages in which modules will perform logging is contradictory to having an additional boolean debug flag here. So you would make this a debug statement here.

DocGarbanzo · 2022-03-20T19:55:50Z

donkeycar/parts/gps.py

+    #
+    parts = gps_str.split(".")
+    degrees_str = parts[0][:-2]
+    minutes_str = parts[0][-2:]


Looking at this again, I think you don't need to do any split here: just

degrees = float(gps_str[:2]) minuts = float(gps_str[2:])

donkeycar/parts/gps.py

DocGarbanzo · 2022-03-20T20:15:21Z

install/envs/mac.yml

  - psutil
  - kivy=2.0.0
  - plotly
  - pyyaml
  - fastai
  - pip:
-    - tensorflow==2.2.0
+    - --no-binary :h5py: h5py==3.1.0
+    - tensorflow==2.5.3


Yes, please revert those change, the Mac conda environment runs well on the CI as well as on my computer. If the RPi install gives a problem then we need to address this differently, as the RPi installs through pip (using setup.py) only.

DocGarbanzo · 2022-04-04T21:12:02Z

setup.py

@@ -49,7 +49,10 @@ def package_files(directory, strip_leading):
          'progress',
          'typing_extensions',
          'pyfiglet',
-          'psutil'
+          'psutil',


We need to add these to 3 libraries to the conda yaml files.

DocGarbanzo · 2022-04-04T21:13:18Z

@Ezward - everything looks good, I think we need to bump the version (rebase from main) and update the conda yaml files.

- reads gps coordinates as tuple (timestamp, longitude, latitude)

- note that currently it is not able to check for buffered chars on the mac; the Serial.in_waiting always returns 0. - this is a problem; it would be better to have the loop continue if no chars are waiting, rather than block. - I will test this on raspberrypi and nano and see if i can get that feature to work

- Serial.in_waiting always returns 0 on mac, but it works on raspberrypi os, so I added it back in. - Added plotting of the waypoints, their bounds and ellipse - Added separate hit tests for bounds, ellipse and axis-aligned standard deviation.

- limit self.debug checks to those that should only happen using the __main__ self test.

- I have successfully used these options directly with pip - I cannot test them directly because conda will not resolve the dependencies

- we will deal with mac install issues in a separate issue

- logger had syntax error - modern gps use 'GNRMC' message, which is standard

- based on multiple of standard deviation

Ezward · 2022-04-09T06:06:44Z

@Ezward - everything looks good, I think we need to bump the version (rebase from main) and update the conda yaml files.

I did that and updated the conda yaml files, but I have not tested these yet.

DocGarbanzo

Looks good to me - need to wait until checks passed.

DocGarbanzo · 2022-04-21T21:02:57Z

setup.py

@@ -24,7 +24,7 @@ def package_files(directory, strip_leading):
    long_description = fh.read()

 setup(name='donkeycar',
-      version="4.3.10",
+      version="4.3.11",


I just merged your other PR which bumped the version to 4.3.11 - can you rebase and force-push?

donkeycar/parts/gps.py

Ezward requested review from zlite and DocGarbanzo March 6, 2022 21:49

Ezward self-assigned this Mar 6, 2022

Ezward mentioned this pull request Mar 6, 2022

Add gps part and integrate into path_follow template #991

Closed

7 tasks

DocGarbanzo reviewed Mar 14, 2022

View reviewed changes

Ezward requested a review from DocGarbanzo March 16, 2022 18:07

DocGarbanzo reviewed Mar 20, 2022

View reviewed changes

Ezward requested a review from DocGarbanzo March 22, 2022 05:08

DocGarbanzo reviewed Apr 4, 2022

View reviewed changes

Ezward and others added 17 commits April 8, 2022 22:57

Add gps.py part

aaa5e94

- reads gps coordinates as tuple (timestamp, longitude, latitude)

add utm dependency for gps part

aaca358

Minor comment change. Now tested on RPi and Jetson Nano

f2c9411

remove explicit setting of logger level

45184ae

add pynmea2 nmea parser for gps part

b15479c

remove redundant check for self.gps

541c495

change most logs to info

b11c7dc

- limit self.debug checks to those that should only happen using the __main__ self test.

fix pip section of install/envs/mac.yml

8d533bf

- I have successfully used these options directly with pip - I cannot test them directly because conda will not resolve the dependencies

revert mac.yml to be same as main

20700d2

- we will deal with mac install issues in a separate issue

rebase against main and change version="4.3.11"

8192e21

small changes based on PR comments

51d774e

fix logger and support ublox 8 and 9 series

d506111

- logger had syntax error - modern gps use 'GNRMC' message, which is standard

removed redundant exception handling

18e52b1

add arg to main to choose size of ellipse

8b2cdd7

- based on multiple of standard deviation

allow one waypoint in test

ce709e6

Ezward force-pushed the 991-gps-part branch from ba579f8 to ce709e6 Compare April 9, 2022 05:59

update conda yaml files with gps dependencies

96dfd37

Ezward requested a review from DocGarbanzo April 9, 2022 16:55

Ezward added 2 commits April 21, 2022 20:21

version="4.3.12"

2c12b6d

Merge branch 'main' into 991-gps-part

2f66e14

DocGarbanzo approved these changes Apr 22, 2022

View reviewed changes

DocGarbanzo merged commit e1083e4 into main Apr 22, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add gps.py part #992

Add gps.py part #992

Ezward commented Mar 6, 2022 •

edited

Ezward commented Mar 7, 2022

DocGarbanzo left a comment

DocGarbanzo Mar 14, 2022

Ezward Mar 16, 2022

DocGarbanzo Mar 14, 2022

Ezward Mar 18, 2022

DocGarbanzo Mar 20, 2022

Ezward Mar 21, 2022 •

edited

DocGarbanzo Mar 14, 2022

Ezward Mar 18, 2022

DocGarbanzo Mar 14, 2022

DocGarbanzo Mar 14, 2022

Ezward Mar 19, 2022

DocGarbanzo Mar 14, 2022

Ezward Mar 19, 2022

DocGarbanzo Mar 14, 2022

Ezward Mar 19, 2022

Ezward Mar 19, 2022

Ezward Mar 19, 2022

DocGarbanzo Mar 20, 2022

Ezward Mar 22, 2022

DocGarbanzo Mar 20, 2022

Ezward Mar 21, 2022

DocGarbanzo Mar 20, 2022

Ezward Mar 21, 2022

DocGarbanzo Mar 20, 2022

DocGarbanzo Mar 20, 2022

DocGarbanzo Mar 20, 2022

DocGarbanzo Apr 4, 2022

DocGarbanzo commented Apr 4, 2022

Ezward commented Apr 9, 2022

DocGarbanzo left a comment

DocGarbanzo Apr 21, 2022

Add gps.py part #992

Add gps.py part #992

Conversation

Ezward commented Mar 6, 2022 • edited

Ezward commented Mar 7, 2022

DocGarbanzo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ezward Mar 21, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DocGarbanzo commented Apr 4, 2022

Ezward commented Apr 9, 2022

DocGarbanzo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ezward commented Mar 6, 2022 •

edited

Ezward Mar 21, 2022 •

edited