atsushisakai / pyroombaadapter Goto Github PK

Hi! Trying to use the send_drive_direct() and send_drive_pwm() methods raise the following exception: NameError: name 'byte_tool' is not defined. It looks like these methods should be using self._get_2_bytes() instead.

[ ] design architecture of getting sensor data
[ ] Implement API to get Sensor Packets

Hi,

I found that in constructor of PyRoombaAdapter a sys.exit(1) call is hard-coded, so you cannot try to reconnect without restart python script. So if serial connect is fail you cannot as program exit with no option.

I think more flexible here is just re-raise some custom exception with human-readable message.

What you think about this?

Hello. Attempting to install PyRoombaAdapter 0.1.4 from pip results in an error:

josh@roomba:~$ sudo pip3 install pyroombaadapter
Looking in indexes: https://pypi.org/simple, https://www.piwheels.org/simple
Collecting pyroombaadapter
  Downloading https://files.pythonhosted.org/packages/2a/a6/beb4c900dfaa642c04364e1de1a1262b02bd7611b7d7a67f4cd4a57db230/pyroombaadapter-0.1.4.tar.gz
    Complete output from command python setup.py egg_info:
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/tmp/pip-install-d7kk3yw9/pyroombaadapter/setup.py", line 25, in <module>
        with open(PROJECT_PATH + "/VERSION", 'r') as fd:
    FileNotFoundError: [Errno 2] No such file or directory: '/tmp/pip-install-d7kk3yw9/pyroombaadapter/VERSION'
    
    ----------------------------------------
Command "python setup.py egg_info" failed with error code 1 in /tmp/pip-install-d7kk3yw9/pyroombaadapter/

Hi. If you try to create a new instance of PyRoombaAdapter without the serial port being present, it raises a SerialException. However, this exception is not defined in pyroombaadapter.py and causes a NameError exception to be raised. This is an easy fix (see commit process1183@8699ffc). After fixing the SerialException definition, I ran into another issue: If opening the serial port fails, then the __del__() method also throws an exception. Here's an example of this:

Python 3.7.3 (default, Jan 22 2021, 20:04:44) 
[GCC 8.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import pyroombaadapter
>>> roomba = pyroombaadapter.PyRoombaAdapter("/dev/doesnotexist")
Cannot find serial port. Plase reconnect it.
Exception ignored in: <function PyRoombaAdapter.__del__ at 0x7f96d2691b70>
Traceback (most recent call last):
  File "/home/josh/projects/forked/PyRoombaAdapter/pyroombaadapter/pyroombaadapter.py", line 86, in __del__
    self._send_cmd(self.CMD["Start"])  # move to passive mode
  File "/home/josh/projects/forked/PyRoombaAdapter/pyroombaadapter/pyroombaadapter.py", line 608, in _send_cmd
    self.serial_con.write(bytes([cmd]))
AttributeError: 'PyRoombaAdapter' object has no attribute 'serial_con'

One possible solution is to check that self.serial_con exists before trying to set the Roomba to passive mode and close the connection in __del__(). Does this seem like a good solution to you? If so, I can include it in the pull request for the SerialException fix.

Local doc looks have it…

https://atsushisakai.github.io/PyRoombaAdapter/API.html

Hi,

I found that numpy.deg2rad(x) in all examples can be replaced to standard library call math.radians(x) and thus avoid heavyweight NumPy dependency in examples at start.

I think it will be easier and faster for fast try and start.

Here is small test for interdependence these functions. Seems work fine:

import math
import numpy as np

from unittest import TestCase


class Deg2Radians(TestCase):
    def testDeg2radians(self):
        self.assertInterdependence(0.0)
        self.assertInterdependence(23.0)
        self.assertInterdependence(45.0)
        self.assertInterdependence(360.0)
        self.assertInterdependence(720.0)
        self.assertInterdependence(-720.0)
        self.assertInterdependence(-90.0)
        self.assertInterdependence(-10.0)
        self.assertInterdependence(-90)
        self.assertInterdependence(-10)

    def assertInterdependence(self, degrees):
        np_rad = np.deg2rad(degrees)
        math_rad = math.radians(degrees)

        self.assertFloat(np_rad)
        self.assertFloat(math_rad)
        self.assertAlmostEqual(math_rad, np_rad)

    def assertFloat(self, value):
        try:
            float(value)
        except ValueError:
            self.fail(f"Value {value} cannot be convert to float")

See: https://docs.python.org/3/library/math.html#math.radians

PS: Thanks for cool project! 🚀