Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added motor error warning mechanism #22

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

albertoramonj
Copy link

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
Copy link

VGonPa commented Oct 21, 2014

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

@arebgun
Copy link
Owner

arebgun commented Oct 22, 2014

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
Copy link

VGonPa commented Oct 23, 2014

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
Copy link
Collaborator

130s commented Dec 27, 2014

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
Copy link

VGonPa commented Jan 8, 2015

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
Copy link
Owner

arebgun commented Feb 24, 2015

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants