Raspberry PI controllable car (code for a 6-year old)

Question

I've been trying to show my 6-year old daughter a way to build and create a simple toy car. We've got a 4-motor chassis kit, Raspberry PI and a motozero shield that can operate these 4 motors. We've put a power bank and a separate battery for the motors:

                                      

                                              _{(The green light on the right is the traffic light)}

Now, as far as coding goes, I've created a simple Car class with forward, backward, forward_left, forward_right, backward_left and backward_right actions. For the demonstration purposes mostly, I've tried to be explicit and violated the DRY principle a lot.

I've also initialized an endless loop listening for keyboard inputs - F would be for forward, B for backwards and S for stop (have not decided which keys would go for those "left-ish" and "right-ish" actions yet).

from RPi import GPIO
from gpiozero import Motor, OutputDevice


class Car:
    def __init__(self):
        self.front_right = Motor(forward=24, backward=27)
        self.front_left = Motor(forward=6, backward=22)

        self.rear_right = Motor(forward=13, backward=18)
        self.rear_left = Motor(forward=23, backward=16)

        OutputDevice(5).on()
        OutputDevice(17).on()
        OutputDevice(25).on()
        OutputDevice(12).on()

    def forward(self):
        self.front_right.forward()
        self.front_left.backward()

        self.rear_right.backward()
        self.rear_left.backward()

    def backward(self):
        self.front_right.backward()
        self.front_left.forward()

        self.rear_right.forward()
        self.rear_left.forward()

    def backward_right(self):
        self.front_right.backward(1)
        self.front_left.forward(1)

        self.rear_right.forward(1)
        self.rear_left.forward(0.2)

    def backward_left(self):
        self.front_right.backward(1)
        self.front_left.forward(1)

        self.rear_right.forward(0.2)
        self.rear_left.forward(1)

    def forward_left(self):
        self.front_right.forward()
        self.front_left.backward(0.2)

        self.rear_right.backward(1)
        self.rear_left.backward(1)

    def forward_right(self):
        self.front_right.forward(0.2)
        self.front_left.backward(1)

        self.rear_right.backward(1)
        self.rear_left.backward(1)

    def stop(self):
        self.front_right.stop()
        self.front_left.stop()

        self.rear_right.stop()
        self.rear_left.stop()


if __name__ == '__main__':
    GPIO.setwarnings(False)
    GPIO.cleanup()

    commands = {
        "f": "forward",
        "b": "backward",
        "s": "stop"
    }
    try:
        car = Car()

        while True:
            command = raw_input()
            getattr(car, commands[command])()
    finally:
        try:
            GPIO.cleanup()
        except:  # ignore cleanup errors
            pass

(I've also mixed up some motors and pins on the board which led to some motors accepting "backwards" as "forward")

It is, of course, difficult to explain this kind of code to a 6-year old, since she still expects a printer to print something when I show her the Python's print() command, but, how would you change this code to make it more understandable for a kid?

I would also appreciate any points about the code organization and quality since the next step would be to control the car "via Bluetooth".

Answer 1

The implementation of forward is horrible. The only correct way of expressing it is to make all four motors go forward. As an analogy, how would you react if your keyboard manufacturer told you "oh sorry, we made a mistake, you have to type every second letter in uppercase to get a lowercase word"? That would be non-intuitive.

The API you provide must be as clean and simple as possible. You don't want your daughter to learn that "programmers make mistakes and are lazy about fixing them, so we have to live with that".

In which ways is your daughter supposed to change the current program? You should probably add some example code for waiting some time, to allow for composed commands like "forward, wait 1s, right, 1s, backward".

Your code seems to crash with an exception when an unexpected key is pressed. That's unacceptable for a robot, since it might harm people or pets when running uncontrolled (see Asimov's laws).

Using raw_input looks as if the user has to press the Enter key to confirm each action. That's too much of a burden. Controlling the car must be simple and immediate, to prevent accidents. How would you feel if your grown-up street car would ask you for confirmation for each step on the gas or brake?

Answer 2

• From my experience, kids understand DRY without explanation, and in fact they are very bored by the repetition.

• which led to some motors accepting "backwards" as "forward"

  This is I am afraid a dire mistake. Fix it as soon as you can, because it may ruin kid's intuition.Even I can't grok why forward method is so asymmetric.

• Do not rely on magic numbers and default argument values. Defaults are really hard to grasp.

All that said, I believe the question is better suited for CSE exchange.

Answer 3

I wrote a Pi+python control program so I had a quick look at what I did. Interestingly I noticed that I seem to have had the same problem with forwards and backwards. I found this comment:

# Forwards and backwards are the same for left and right
# The trick is to reverse the motor connections for one  side :-) 

Thus I solved it by swapping the connections for both motors on one side. I could tell you where the code it but it is on a website which has my product and I don't want to advertise. It uses a different motor controller but the principle is the same. Here is the core code:

# Main program

# Get the curses screen
screen = curses.initscr()

..... <omitted code to set up the I/O>

# Tell user what to expect
# in curses print does not work anymore. use addstr
screen.addstr("<prouduct name> example program for Python\n")
screen.addstr("Use numeric keypad keys control\n")
screen.addstr("Left  Both  Right\n")
screen.addstr("     Forward     \n")
screen.addstr("  7     8     9  \n")
screen.addstr("                 \n")
screen.addstr("  4    Stop   6  \n")
screen.addstr("                 \n")
screen.addstr("  1     2     3  \n")
screen.addstr("     Reverse     \n")
screen.addstr("Left  Both  Right\n")
screen.addstr("Don't forget to set numlock on!!!!\n")
screen.addstr("Use Q or q to quit\n")
screen.addstr("\n")


run = 1
while run==1 :
  key = screen.getch() # Key?
  if key==ord('q') :
    run = 0 # stop running

  if key==ord('1') : 
     gb.move_brushed(BOARD,LEFT,BACKW) # Left backwards 

  if key==ord('2') : 
     gb.move_brushed(BOARD,LEFT,BACKW)  # Left backwards 
     gb.move_brushed(BOARD,RIGHT,BACKW) # Right backwards 

  if key==ord('3') : 
     gb.move_brushed(BOARD,RIGHT,BACKW) # Right backwards 

  if key==ord('4') :
     gb.move_brushed(BOARD,LEFT,STOP) # Left stop 

  if key==ord('5') : 
     gb.move_brushed(BOARD,LEFT,STOP)  # Left stop 
     gb.move_brushed(BOARD,RIGHT,STOP) # Right stop 

  if key==ord('6') :
     gb.move_brushed(BOARD,RIGHT,STOP) # Right stop 

  if key==ord('7') :
     gb.move_brushed(BOARD,LEFT,FORWD) # Left forwards 

  if key==ord('8') :
     gb.move_brushed(BOARD,LEFT,FORWD)  # Left forwards 
     gb.move_brushed(BOARD,RIGHT,FORWD) # Right forwards 

  if key==ord('9') :
     gb.move_brushed(BOARD,RIGHT,FORWD) # Right forwards 

# on exist stop everything
gb.emerg_stop()
# Set terminal behaviour normal again
curses.endwin()

Answer 4

On a different note, in order to not have to press enter after every command you could add the following modification to the main part of the code to use pygame although this would make it less understandable so I would definitely keep both versions.

if __name__ == '__main__':
    GPIO.setwarnings(False)
    GPIO.cleanup()

    try:
        car = Car()
        import pygame

        while True:
            events = pygame.event.get()
            for event in events:
                if event.type == pygame.KEYDOWN:
                    if event.key == pygame.K_UP or event.key == pygame.K_w:
                        car.forward()
                    elif event.key == pygame.K_LEFT or event.key == pygame.K_a:
                        car.forward_left()
                    elif event.key == pygame.K_RIGHT or event.key == pygame.K_d:
                        car.forward_right()
                    elif event.key == pygame.K_DOWN or event.key == pygame.K_s:
                        car.backward()
                    elif event.key == pygame.K_SPACE:
                        car.stop()
    finally:
        try:
            GPIO.cleanup()
        except:  # ignore cleanup errors
            pass

Where wasd or arrow keys controls its movement and space stops it.

With thanks to @Haz on https://stackoverflow.com/questions/16044229/how-to-get-keyboard-input-in-pygame

Answer 5

I think the advice from import this will help you here, especially "Explicit is better than implicit", "Simple is better than complex", "Flat is better than nested" and "Readability counts." In particular, I would note that you are creating a Car class which can only be used as a singleton. But Python modules are already perfectly valid singleton classes, so you can eliminate a level of abstraction and nesting by implementing everything directly at the module level.

I would also agree with others that you must fix the forward/backward problems, and it's a good idea to always specify the motor speed rather than relying on defaults. I like your event stack approach, but I would reference functions directly (not by name) and I would add an extra step to make it easier to see what's happening.

This gives simpler code, like this:

from RPi import GPIO
from gpiozero import Motor, OutputDevice

# setup GPIO
GPIO.setwarnings(False)
GPIO.cleanup()
OutputDevice(5).on()
OutputDevice(17).on()
OutputDevice(25).on()
OutputDevice(12).on()

# setup motor references (with correct forward/backward settings)
front_right = Motor(forward=24, backward=27)
front_left = Motor(forward=22, backward=6)
rear_right = Motor(forward=18, backward=13)
rear_left = Motor(forward=16, backward=23)

# define available commands
def forward(self):
    front_right.forward(1)
    front_left.forward(1)
    rear_right.forward(1)
    rear_left.forward(1)

def backward(self):
    front_right.backward(1)
    front_left.backward(1)
    rear_right.backward(1)
    rear_left.backward(1)

def backward_right(self):
    front_right.backward(1)
    front_left.backward(1)
    rear_right.backward(1)
    rear_left.backward(0.2)

def backward_left(self):
    front_right.backward(1)
    front_left.backward(1)
    rear_right.backward(0.2)
    rear_left.backward(1)

def forward_left(self):
    front_right.forward(1)
    front_left.forward(0.2)
    rear_right.forward(1)
    rear_left.forward(1)

def forward_right(self):
    front_right.forward(0.2)
    front_left.forward(1)
    rear_right.forward(1)
    rear_left.forward(1)

def stop(self):
    front_right.stop()
    front_left.stop()
    rear_right.stop()
    rear_left.stop()

commands = {
    "f": forward,
    "b": backward,
    "s": stop
}

try:
    while True:
        # better to use another library that can use arrow keys and maybe spacebar for stop
        command = raw_input() 
        func = commands[command]
        func()
finally:
    try:
        GPIO.cleanup()
    except:  # ignore cleanup errors
        pass

If you want a DRYer approach, you could do this (at the cost of more abstraction):

from RPi import GPIO
from gpiozero import Motor, OutputDevice

# setup GPIO
GPIO.setwarnings(False)
GPIO.cleanup()
OutputDevice(5).on()
OutputDevice(17).on()
OutputDevice(25).on()
OutputDevice(12).on()

# setup motor references (with correct forward/backward settings)
motors = dict(
    front_right=Motor(forward=24, backward=27),
    front_left=Motor(forward=22, backward=6),
    rear_right=Motor(forward=18, backward=13),
    rear_left=Motor(forward=16, backward=23)
)

# define available commands (dicts of speeds for each motor)
commands = {
    "f": dict(front_right=1, front_left=1, rear_right=1, rear_left=1),
    "b": dict(front_right=-1, front_left=-1, rear_right=-1, rear_left=-1),
    "s": dict(front_right=0, front_left=0, rear_right=0, rear_left=0),
}

def set_motor_speed(motor, speed):
    if speed < 0:
        motor.backward(-speed)
    elif speed == 0:
        motor.stop()
    else:
        motor.forward(speed)

def run_command(command):
    settings=commands[command]
    for motor_name, speed in settings.items():
        set_motor_speed(motors[motor_name], speed)

try:
    while True:
        # better to use another library that can use arrow keys and maybe spacebar for stop
        command = raw_input() 
        run_command(command)
finally:
    try:
        GPIO.cleanup()
    except:  # ignore cleanup errors
        pass

Answer 6

class Car:
    def __init__(self):
        self.front_right = Motor(forward=24, backward=27)
        self.front_left = Motor(forward=22, backward=6)

        self.rear_right = Motor(forward=18, backward=13)
        self.rear_left = Motor(forward=16, backward=23)

This modification to the __init__ method should mean that it doesn't matter that you muddled the pins up because I just reversed the ones which are muddled up. NOTE: It still needs the bit of code to switch them on. Also, it may be beneficial if this is to try and help someone to learn coding to add comments to detail what everything does.