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

Multi-group support for MotoPlus motion driver #43

Merged
merged 8 commits into from Sep 8, 2014
Merged

Multi-group support for MotoPlus motion driver #43

merged 8 commits into from Sep 8, 2014

Conversation

ted-miller
Copy link
Contributor

See changelog in ted-miller/motoman branch.

Add support for multiple control groups.
- Support for SDA robots, or multiple individual robots and/or external
axes.
- Add new command message for controlling up to 4 groups.
- Add new position-feedback message to send all group data.

Add compatibility for DX200 controller.

Convert MotoPlusIDE projects into Visual Studio solution.
- Maintained legacy compatibility for MPIDE.

Improve I/O feedback signals.
- Allocate additional signals for future expansion.
- Add more cases where feedback signals are used.

Improve error handling
- Add additional text and I/O feedback in error cases
Primitive I/O support
> Added custom Motoman-specific message for reading and writing a single
I/O point in the controller.
> Note: Write-support is limited to only certain addresses in the robot
controller.  See wiki for details.

Fixed multiple-arm support for the DX100 controller.
Corrected the ROS_MSG_MOTO_MOTION_REPLY when replying to
ROS_MSG_JOINT_TRAJ_PT_FULL_EX.  A motion-reply message will be sent for
each control group affected by the multi-group-motion message.  The
motion-reply will correctly indicate the control group index for what it
represents.
* Corrected ROS_MSG_JOINT_FEEDBACK_EX message.
- This message will only be sent if the controller has more than one
control-group.
- Single arm systems will not send this message.
No change actual job; just to folder structure.
@@ -94,10 +102,10 @@ typedef struct // ROS_MSG_ROBOT_STATUS = 13
int in_motion; // Is currently executing a motion command: -1=Unknown, 1=True, 0=False
int mode; // Controller/Pendant mode: -1=Unknown, 1=Manual(TEACH), 2=Auto(PLAY)
int motion_possible; // Is the controller ready to receive motion: -1=Unknown, 1=ENABLED, 0=DISABLED
} SmBodyRobotStatus;
} __attribute__((__packed__));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't seen this in a long time ;). Why was it needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This guarantees consistent structure alignment on all three supported robot controllers. The three different compilers apply padding slightly differently. During development, I found that at least one of these structs (don't remember which) behaved differently between the FS and DX. So, I went through and ensured everyone of them is packed without any extra padding between struct members.

@shaun-edwards
Copy link
Member

@ted-miller, Thanks for the contribution.

There are some changes that are being marked due to differences in spacing (tabs vs spaces). I can set the diff to ignore spacing, but the view is still messed up with some code running of the right hand side. Is this easy for you to fix/adjust. If so, can you do another commit (it will automatically get added to the PR).

See other comments/questions inline.

@ted-miller
Copy link
Contributor Author

Can you point to a specific example of a file that is showing these white-space differences? I should be able to convert the tabs to spaces (even though it goes against every fiber of my being to not use tabs... that's almost as bad as putting the curly bracket on the same line as an IF statement)

@ted-miller
Copy link
Contributor Author

I just found some examples in Controller.h and Controller.c.

However, I would argue that the tabs should stay. I put them there to fix some of the display issues in the MotoPlus environment (either Visual Studio or M+IDE). Since MotoPlus doesn't allow much flexibiilty with your choice of IDE's, I think it works out well.
Agree or disagree?

@shaun-edwards
Copy link
Member

Tabs vs spaces....wars have been started over smaller disagreements. I agree with you.

@shaun-edwards
Copy link
Member

One additional question. Are you concerned that the IO calls could add latency to motion streaming (since they share the same communications channel)? This is more for my info.

@ted-miller
Copy link
Contributor Author

Yes. I have personally seen this become a problem when exchanging the amount of I/O that is typical for an industrial work-cell.
I added a basic message in there for triggering a single I/O point like opening a gripper (I'm not concerned about that). But for a production system, I would much rather see the ROS node use a standard I/O fieldbus technology (Ethernet/IP, Profinet, Modbus TCP) on a dedicated channel, rather than implementing new ROS-I messages just for I/O.

@shaun-edwards
Copy link
Member

+1, Waiting to merge until PR ros-industrial/industrial_core#82 is approved.

@thiagodefreitas
Copy link
Contributor

@ted-miller, as requested by @shaun-edwards on my PR, please change the JOINT_TRAJ_PT_FULL_EX and JOINT_FEEDBACK_EX to the MOTOMAN range.

In my opinion, the following values could be used, as you have already suggested in the past:

ROS_MSG_MOTO_JOINT_TRAJ_PT_FULL_EX with a value of 2016.
The new feedback message is ROS_MSG_MOTO_JOINT_FEEDBACK_EX with a value of 2017.

Thanks.

@ted-miller
Copy link
Contributor Author

@thiagodefreitas
Done. See updates in v1.2.4

shaun-edwards added a commit that referenced this pull request Sep 8, 2014
Multi-group support for MotoPlus motion driver
@shaun-edwards shaun-edwards merged commit b0fe2e5 into ros-industrial:hydro-devel Sep 8, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants