Various problems with mech_driver.py

[dfarrell07]

Recent changes to mech_driver introduced some minor problems.

• Most importantly, there are mixed tabs and spaces causing invalid syntax. All tabs should be replaced with four spaces, as specified in PEP8.
• In the init method, max_speed is defined in the context of the init method only. In order to access it outside of that method, use self.max_speed.

    def __init__(self):
        """Run superclass's init."""
        super(MechDriver, self).__init__()
        max_speed = 10

• Inline documentation should have a space between the pound and the text.

    #Prevent invalid duty cycle values.

• There shouldn't be more than one blank line separating logically distinct sections of code in a method.

        if back_right_ds < 0:
          back_right_ds = 0

        #Determine direction of wheel.
        #Bool will determine value of direction pin.
        if front_left > 0:
          front_left_forward = True

• No line should be longer than 79 characters.

    #Todo for Ahmed: Run tests to find out what max_speed should be, if it should just be 'speed'.

• I suggest the following format for TODOs. The name in the parens should be the person who made the TODO.

    # TODO(dfarrell07): Refactor this to not use a global.

• What's the intention of this code? Currently, it's just multiplying each value by 10 and then rounding it down.

        front_left_ds = floor(front_left / self.max_speed * 100)
        front_right_ds = floor(front_right / self.max_speed * 100)
        back_left_ds = floor(back_left / self.max_speed * 100)
        back_right_ds = floor(back_right / self.max_speed * 100) 

For example, here's a quick test in an interactive Python shell as well as the logger output (logging statements added by me, not in the repo).

>>> import driver.mech_driver as mdriver
>>> driver = mdriver.MechDriver()                                                                                  
>>> driver.basic_move(50, 0, 0)

2013-07-29 02:01:44,617 | DEBUG | lib.py | get_logger | 140 | Logger built
2013-07-29 02:01:44,618 | DEBUG | driver.py | __init__ | 15 | Driver has logger
2013-07-29 02:01:44,618 | DEBUG | driver.py | __init__ | 18 | Driver has config
2013-07-29 02:02:02,572 | DEBUG | mech_driver.py | basic_move | 55 | Speed: 50, angle 0, rotate speed 0
2013-07-29 02:02:02,573 | DEBUG | mech_driver.py | basic_move | 64 | front_left: 35.3553390593
2013-07-29 02:02:02,573 | DEBUG | mech_driver.py | basic_move | 65 | front_right: 35.3553390593
2013-07-29 02:02:02,573 | DEBUG | mech_driver.py | basic_move | 66 | back_left: 35.3553390593
2013-07-29 02:02:02,574 | DEBUG | mech_driver.py | basic_move | 67 | back_right: 35.3553390593
2013-07-29 02:02:02,574 | DEBUG | mech_driver.py | basic_move | 76 | front_left_ds: 353.0
2013-07-29 02:02:02,574 | DEBUG | mech_driver.py | basic_move | 77 | front_right_ds: 353.0
2013-07-29 02:02:02,574 | DEBUG | mech_driver.py | basic_move | 78 | back_left_ds: 353.0
2013-07-29 02:02:02,575 | DEBUG | mech_driver.py | basic_move | 79 | back_right_ds: 353.0
2013-07-29 02:02:02,575 | DEBUG | mech_driver.py | iowrite | 39 | IO write: motor: front_left, ds: 100
2013-07-29 02:02:02,575 | DEBUG | mech_driver.py | iowrite | 39 | IO write: motor: front_right, ds: 100
2013-07-29 02:02:02,576 | DEBUG | mech_driver.py | iowrite | 39 | IO write: motor: back_left, ds: 100
2013-07-29 02:02:02,576 | DEBUG | mech_driver.py | iowrite | 39 | IO write: motor: back_right, ds: 100

• Note that the following code can be simplified to front_left_ds = min(front_left_ds, 100). The same idea can be applied to all of the similar conditionals in the code. It may also be more elegant to store these values in a dict. You would then be able to iterate over all the vars, repeating this check for each. Passing them to iowrite could also be a single call with the full dict as a param. Just a thought, very low priority.

        if front_left_ds > 100:
          front_left_ds = 100

• Generally, make sure you at least do the most basic test (execute your code) before committing. The indentation and self.max_speed problems would have thrown errors at runtime.

[dfarrell07]

Also, I suggest running check_pep8.sh script before committing. Its output explodes if the repo contains anything like the PEP8 problems above.

daniel:~/robot/current/bot$ ./scripts/check_pep8.sh                                                                                                                                                         [69/1834]
./driver/mech_driver.py:26:5: W191 indentation contains tabs
./driver/mech_driver.py:26:5: E101 indentation contains mixed spaces and tabs
./driver/mech_driver.py:40:1: W191 indentation contains tabs
./driver/mech_driver.py:40:1: E101 indentation contains mixed spaces and tabs
./driver/mech_driver.py:40:1: W293 blank line contains whitespace
./driver/mech_driver.py:42:5: E303 too many blank lines (2)
./driver/mech_driver.py:59:9: E303 too many blank lines (2)
./driver/mech_driver.py:59:39: E225 missing whitespace around operator
./driver/mech_driver.py:59:42: E225 missing whitespace around operator
./driver/mech_driver.py:59:51: E225 missing whitespace around operator
./driver/mech_driver.py:60:40: E225 missing whitespace around operator
./driver/mech_driver.py:60:43: E225 missing whitespace around operator
./driver/mech_driver.py:60:53: E225 missing whitespace around operator
./driver/mech_driver.py:61:38: E225 missing whitespace around operator
./driver/mech_driver.py:61:41: E225 missing whitespace around operator
./driver/mech_driver.py:61:51: E225 missing whitespace around operator
./driver/mech_driver.py:62:39: E225 missing whitespace around operator
./driver/mech_driver.py:62:42: E225 missing whitespace around operator
./driver/mech_driver.py:62:51: E225 missing whitespace around operator
./driver/mech_driver.py:63:1: W191 indentation contains tabs
./driver/mech_driver.py:63:1: E101 indentation contains mixed spaces and tabs
./driver/mech_driver.py:63:1: W293 blank line contains whitespace
./driver/mech_driver.py:64:1: W191 indentation contains tabs
./driver/mech_driver.py:64:1: E101 indentation contains mixed spaces and tabs
./driver/mech_driver.py:65:80: E501 line too long (95 characters)
./driver/mech_driver.py:65:1: W191 indentation contains tabs
./driver/mech_driver.py:65:1: E101 indentation contains mixed spaces and tabs
./driver/mech_driver.py:66:1: W191 indentation contains tabs
./driver/mech_driver.py:66:1: E101 indentation contains mixed spaces and tabs
./driver/mech_driver.py:67:1: W191 indentation contains tabs
./driver/mech_driver.py:67:1: E101 indentation contains mixed spaces and tabs
./driver/mech_driver.py:68:1: W191 indentation contains tabs
<snip>

[AhmedSamara]

Sorry about that. I worked on fixing these but haven't gotten around to commiting them.

I've been having trouble with git bash so I finally got around to installing a linux dual boot, but I'm still having some trouble getting everything working on that.

Also, I suggest running check_pep8.shhttps://github.com/NCSUhardware/bot/blob/master/scripts/check_pep8.shscript before committing. Its output explodes if the repo contains anything like the PEP8 problems above.

daniel:~/robot/current/bot$ ./scripts/check_pep8.sh

[69/1834] ./driver/mech_driver.py:26:5: W191 indentation contains tabs ./driver/mech_driver.py:26:5: E101 indentation contains mixed spaces and tabs ./driver/mech_driver.py:40:1: W191 indentation contains tabs ./driver/mech_driver.py:40:1: E101 indentation contains mixed spaces and tabs ./driver/mech_driver.py:40:1: W293 blank line contains whitespace ./driver/mech_driver.py:42:5: E303 too many blank lines (2) ./driver/mech_driver.py:59:9: E303 too many blank lines (2) ./driver/mech_driver.py:59:39: E225 missing whitespace around operator ./driver/mech_driver.py:59:42: E225 missing whitespace around operator ./driver/mech_driver.py:59:51: E225 missing whitespace around operator ./driver/mech_driver.py:60:40: E225 missing whitespace around operator ./driver/mech_driver.py:60:43: E225 missing whitespace around operator ./driver/mech_driver.py:60:53: E225 missing whitespace around operator ./driver/mech_driver.py:61:38: E225 missing whitespace around operator ./driver/mech_driver.py:61:41: E225 missing whitespace around operator ./driver/mech_driver.py:61:51: E225 missing whitespace around operator ./driver/mech_driver.py:62:39: E225 missing whitespace around operator ./driver/mech_driver.py:62:42: E225 missing whitespace around operator ./driver/mech_driver.py:62:51: E225 missing whitespace around operator ./driver/mech_driver.py:63:1: W191 indentation contains tabs ./driver/mech_driver.py:63:1: E101 indentation contains mixed spaces and tabs ./driver/mech_driver.py:63:1: W293 blank line contains whitespace ./driver/mech_driver.py:64:1: W191 indentation contains tabs ./driver/mech_driver.py:64:1: E101 indentation contains mixed spaces and tabs ./driver/mech_driver.py:65:80: E501 line too long (95 characters) ./driver/mech_driver.py:65:1: W191 indentation contains tabs ./driver/mech_driver.py:65:1: E101 indentation contains mixed spaces and tabs ./driver/mech_driver.py:66:1: W191 indentation contains tabs ./driver/mech_driver.py:66:1: E101 indentation contains mixed spaces and tabs ./driver/mech_driver.py:67:1: W191 indentation contains tabs ./driver/mech_driver.py:67:1: E101 indentation contains mixed spaces and tabs ./driver/mech_driver.py:68:1: W191 indentation contains tabs

— Reply to this email directly or view it on GitHubhttps://github.com/NCSUhardware/bot/issues/2#issuecomment-21701691 .

[dfarrell07]

I'm sure quite a few team members would be able to help out with any Linux problems you're having. Feel free to reach out to us. :)

[dfarrell07]

I believe that all of these problems have been corrected. Good job, closing issue.