search

arebgun / dynamixel_motor

ROS stack for interfacing with Robotis Dynamixel line of servo motors.
BSD 3-Clause "New" or "Revised" License
70 stars 169 forks source link

Added motor error warning mechanism #22

Open albertoramonj opened 10 years ago

albertoramonj commented 10 years ago

Hi Anton,

We've added a mechanism to warn high level nodes if there is any errors with the servos (now just FatalErrorCodeErrors, such as an overload error). We have created a new custom message named MotorError.msg and added the code necessary to publish the topic with the error info.

MotorError.msg

string error_type            # error type
string error_message         # error message
string extra_info            # extra info such a joint number, port...

dynamixel_serial_proxy.py

except dynamixel_io.FatalErrorCodeError, fece:
                    rospy.logerr(fece)
                    me = MotorError()
                    me.error_type = "FatalErrorCodeError"
                    me.error_message = fece.message
                    me.extra_info = fece.message.split(' ')[3][1:] # get the servo_id
                    pub = rospy.Publisher('dynamixel_motor_errors', MotorError, queue_size=None)
                    pub.publish(me)

Best,

Alberto.

VGonPa commented 10 years ago

Hi @arebgun did you check this PR? We would like to merge the changes in your repo.

arebgun commented 10 years ago

Hi @VGonPa, I would really like to avoid merging extraneous stuff like formatting changes. Can you prepare a more targeted pull request that just adds the new features? Thanks!

VGonPa commented 10 years ago

Hi,

the chages are made to make the code more compliant with PEP8.

If you want we can modify the push request to make it without the PEP8 modifications and later on make another PR to upload the PEP8 changes.

130s commented 9 years ago

I think what @arebgun wanted to mention might have been that the commits should be separated per code style change and functionality change -- mixing those 2 into a single commit makes the review and code tracking in the future really difficult. Ref. http://stackoverflow.com/a/2049159/577001

VGonPa commented 9 years ago

OK, it makes complete sense to me. If you agree, that's what I'm going to do: first I will commit the PEP8 related changes, and once they are merged I will send the other ones.

arebgun commented 9 years ago

If we could just skip the PEP8 formatting changes that would be preferable, I really don't care for 79 character line limit among other things.