18 Motor, 6 Legged robot walking using Python / ROS

I have been using ROS alongside Python to enable my built robot to walk. I am aware that my code needs some improvement such as classes etc. This is my first Python / ROS program so any help and pointers would be hugely appreciated.

```
#!/usr/bin/env python
#

## launch start_robot_controller.launch before running this.

import roslib; roslib.load_manifest('my_dynamixel_tutorial')
import rospy
import math
import operator

import sys
import select

#import msvcrt
#import pygame

import termios, sys, os
TERMIOS = termios

from std_msgs.msg import Float64
from dynamixel_msgs.msg import MotorStateList
from dynamixel_msgs.msg import JointState
from geometry_msgs.msg import Twist

## define the command publishers for joints, Legs are named as: L - left; R - Right; M - middle; R - rear. each leg has three joints, 1,2,3
pub_LF1 = rospy.Publisher("/LF1/command", Float64, latch=True)
pub_LF2 = rospy.Publisher("/LF2/command", Float64, latch=True)
pub_LF3 = rospy.Publisher("/LF3/command", Float64, latch=True)
pub_LM1 = rospy.Publisher("/LM1/command", Float64, latch=True)
pub_LM2 = rospy.Publisher("/LM2/command", Float64, latch=True)
pub_LM3 = rospy.Publisher("/LM3/command", Float64, latch=True)
pub_LR1 = rospy.Publisher("/LR1/command", Float64, latch=True)
pub_LR2 = rospy.Publisher("/LR2/command", Float64, latch=True)
pub_LR3 = rospy.Publisher("/LR3/command", Float64, latch=True)
pub_RF1 = rospy.Publisher("/RF1/command", Float64, latch=True)
pub_RF2 = rospy.Publisher("/RF2/command", Float64, latch=True)
pub_RF3 = rospy.Publisher("/RF3/command", Float64, latch=True)
pub_RM1 = rospy.Publisher("/RM1/command", Float64, latch=True)
pub_RM2 = rospy.Publisher("/RM2/command", Float64, latch=True)
pub_RM3 = rospy.Publisher("/RM3/command", Float64, latch=True)
pub_RR1 = rospy.Publisher("/RR1/command", Float64, latch=True)
pub_RR2 = rospy.Publisher("/RR2/command", Float64, latch=True)
pub_RR3 = rospy.Publisher("/RR3/command", Float64, latch=True)

## leg states. each leg has three state

So many globals

It doesn't seem like you quite realise what global does, so I'll explain how Python's scope works. When you call on a name, Python will search for it in the current scope it's in, but if it can't find it there it will search the next highest level of scope, to check there too. So, for example this code works fine:

foo = "I am global"

def bar():
    print foo

bar()
# I am global

foo is not declared as global, but Python will look for it in the global space after it realises that it's not declared inside bar.

Note, however, that this doesn't work if you try to set foo inside of bar but reference it before you set it. So this will work:

foo = "I am global"

def bar():
    foo = "I am local"
    print foo

bar()
# I am local

But this will not:

foo = "I am global"

def bar():
    print foo
    foo = "I am local"

bar()
# UnboundLocalError: local variable 'foo' referenced before assignment

So while you can read from any scope, it's only when you need to write to it that global is necessary. In the first case above, where foo is set to "I am local", the global foo is still it's original "I am global" value. If you want to change foo's value in the global scope that's where you need to call global foo.

foo = "I am global"

def bar():
    global foo
    foo = "I am redefined"
    print foo

bar()
# I am redefined
print foo
# I am redefined

Note that you don't need to call global foo outside of bar. In fact you never need to call global on anything in the global space. What global actually does is tell Python to use the reference to this name from the global space. When you're already in the global space, that's actually unnecessary.

Now that you know how they work... don't use them

People commonly recommend that you don't use global, as it's a messy way of circumventing scoping rules. A more robust solution is almost always to use a class, and this does fit your case.

If you made a class to hold your values and functions, then you'd just have class or instance attributes that are much easier to access than a series of loose global variables. You can also much clearer separate out what part of your code is concerned with what value/s.

Here's a simple example of how you might structure a class to take foo instead of using global:

class Example:
    def __init__(self, foo):
        self.foo = foo

    def bar(self):
        print self.foo

an_example = Example("Class based foo")
print an_example.foo
# Class based foo
an_example.bar()
# Class based foo

This is much simpler. Now foo is clearly paired with its instance of an_example. You can read it by calling self.foo and know that you have the right foo. It's also easy to change both inside and outside the instance. For example:

an_example.foo = "Easy to change"
print an_example.foo
# Easy to change
an_example.bar()
# Easy to change

If you wanted a value not to be tied to separate instances you can do that too (eg. if you wanted the class itself to have a counter), you can do that too with a class attribute:

class Example:
    foo = "I am foo"

    def bar(self):
        print Example.foo

This works very similarly, it can be accessed through the class name as well as the function:

an_example = Example()
an_example.bar()
# I am foo
print Example.foo
# I am foo

You can then update the class value itself, and the instance will reflect that:

Example.foo += " and I'm changeable."
print Example.foo
# I am foo and I'm changeable.
an_example.bar()
# I am foo and I'm changeable.

Data storage

Also on the data front, you have a lot of separate variables that seem like they should be a dictionary or list instead. Like your series of pub or p values. You should really look at how to organise your data neater as that will trickle down to much more readable code, like being able to loop over a list or dictionary rather than having to have a series of identical lines for each individual value.

StackExchange Code Review Q#117074, answer score: 6