MotoROS v1.8.1 #227

gavanderhoorn · 2018-06-26T12:56:16Z

See commit comments.

Take execution status into account as well when determining the value for the 'in_motion' field of the Robot Status message.

This fixes velocities being reported by the driver for axes that are not part of the motion group.

This should only be applied when necessary, and DX200 and YRC1000 already decouple this earlier.

gavanderhoorn · 2018-06-26T12:56:32Z

@EricMarcil: fyi.

gavanderhoorn · 2018-06-26T13:37:54Z

Note that we'll need to update the links on the wiki again after this gets merged (because of the new binaries). See #220.

gavanderhoorn

Haven't tested the new binaries yet, but here are some comments.

gavanderhoorn · 2018-06-26T14:46:06Z

motoman_driver/MotoPlus/CtrlGroup.c

+			pulseSpeed[dest_axis] -= (int)(pulseSpeed[src_axis] * corr->fCorrectionRatio);
+		}
+	}
+


Could this for be merged with the one directly above it? Would avoid iterating over the pulseSpeed array again.

If you look closely, the axis correction makes reference to another source axis pulseSpeed. Granted that the source axis is normally a previous axis but there is a possibility that it wouldn't. So it is safer to calculate the pulseSpeed for all axes before doing the correction.

gavanderhoorn · 2018-06-26T14:48:02Z

motoman_driver/MotoPlus/Controller.c

+		{
+			//Check group number valid
+			if (!Ros_Controller_IsValidGroupNo(controller, groupNo))
+				continue;


Just checking: if all groupNo are invalid, it immediately means that this function returns FALSE. I'm not sure a robot with all groups being invalid can be in motion, but this is on purpose, correct?

That is correct and intentional. If all groups are invalid, there really nothing to move.

gavanderhoorn · 2018-06-26T14:49:12Z

motoman_driver/MotoPlus/CtrlGroup.c

@@ -455,6 +457,8 @@ BOOL Ros_CtrlGroup_GetTorque(CtrlGroup* ctrlGroup, double torqueValues[MAX_PULSE
 	memset(dst_trq.data, 0, sizeof(MP_TRQCTL_DATA));
 	dst_trq.unit = TRQ_NEWTON_METER; //request data in Nm

+	memset(&dst_vel, 0x00, sizeof(MP_GRP_AXES_T));


style: the memset(..) above this one uses a plain, decimal, 0.

gavanderhoorn · 2018-06-26T14:50:08Z

motoman_driver/MotoPlus/IoServer.c

@@ -97,6 +97,7 @@ void Ros_IoServer_StartNewConnection(Controller* controller, int sd)
 			controller->sdIoConnections[connectionIndex] = INVALID_SOCKET;
 			controller->tidIoConnections[connectionIndex] = INVALID_TASK;
 			Ros_Controller_SetIOState(IO_FEEDBACK_FAILURE, TRUE);
+			mpSetAlarm(8004, "MOTOROS FAILED TO CREATE TASK", 7);


Is the 7 a nr with a special significance? Fi, 7 indicates the IoServer task could not be created?

If so: should we create constants for them, to avoid the magic nrs?

And the same comment for the other mpSetAlarm(..) invocations for task creation failure.

Yes, I've given each mpSetAlarm a different subcode (pretty much in the order they are created), so that if the alarm 8004 occurs, it could help identify which task wasn't started. Generally speaking, I doubt that we would dig down to that level because whenever that 8004 occurs, it's probably because of parameters settings or you have multiple versions of the MotoRos running. So I didn't go as far as creating constants for them.

gavanderhoorn · 2018-06-26T14:50:53Z

motoman_driver/MotoPlus/MotionServer.c

@@ -1552,7 +1556,7 @@ int Ros_MotionServer_GetQueueCnt(Controller* controller, int groupNo)
 	}

 	printf("ERROR: Unable to access queue count.  Queue is locked up!\r\n");
-	return -1;
+	return ERROR;


gavanderhoorn · 2018-06-26T15:20:41Z

motoman_driver/MotoPlus/MotionServer.c

 			return TRUE;
+		else if (qCnt == ERROR)


This could be just an if, right (because of the returns)? Seeing an else if in isolation without an else always looks weird to me.

Yes, it could. That change was made by @ted-miller. Personally, I tend to build the if, else if, else structure based on the conditions that is to be consider (in this case the qCnt value). It's independently of what occurs after the condition is identified. So I tend to agree with the way it is.

gavanderhoorn · 2018-06-26T15:50:59Z

Checked this on our SIA20F+linear rail.

This PR definitely improves the in_motion situation. in_motion stays true for the majority of the motion, and then goes to false three (3) JointFeedback messages before the robot has come to a complete stop. Joint velocity is also > 0 for the three JointFeedback messages after in_motion has changed to false.

I haven't checked, but that could well be within the START_MAX_PULSE_DEVIATION that @EricMarcil mentions in #219 (comment) (robot was moving rather slowly along the rail, and 3 msgs at 50 Hz is 75ms).

gavanderhoorn · 2018-06-26T17:44:35Z

@EricMarcil: I've added some comments.

gavanderhoorn · 2018-06-26T20:15:49Z

@alemme / @andreaskoepf: do you see any chance of testing fe26038 perhaps on your SDA?

It would seem to address at least some of the comments by @andreaskoepf in #219.

andreaskoepf · 2018-06-28T06:21:11Z

@EricMarcil Great to see some improvements here!

@gavanderhoorn We will defiintely test this during the next two weeks. (Currently we are in final preparations for Rosvita v0 release (which hopefully will be the best or one of the best ROS GUIs for controlling the SDA). I plan to sync our Motoman driver with this repo in the July timeframe.)

gavanderhoorn · 2018-06-28T08:03:01Z

@andreaskoepf wrote:

We will defiintely test this during the next two weeks. (Currently we are in final preparations for Rosvita v0 release (which hopefully will be the best or one of the best ROS GUIs for controlling the SDA). I plan to sync our Motoman driver with this repo in the July timeframe.)

Ok, good.

In the meantime I'll go ahead and merge this, as it solves some of the other issues we have with goals being flagged as completed too early.

If anything pops up during your testing please open a new issue.

gavanderhoorn · 2018-06-28T08:10:53Z

I'm not in the habit of doing this btw, but the test in this case would be in addition to the verification that has already been done by me and @ted-miller and @EricMarcil (at Motoman), and is also more a test to see whether the situation that you described in #219 has improved.

gavanderhoorn · 2018-06-28T08:11:39Z

@EricMarcil: as this PR was opened by me, I need one other maintainer to review this PR and approve it. Could you do that?

gavanderhoorn · 2018-06-28T14:21:20Z

Thanks for the review @EricMarcil 👍

Overload is not used by any part of the JTA. Additionally, it was checking only the status of the fourth motion group, which is not a correct way to determine motion completion for the entire robot (at least not until 'in_motion' was fixed in ros-industrial#227). If needed, we can bring it back by reverting this commit.

EricMarcil added 6 commits June 26, 2018 14:41

driver: fix 'in_motion' robot status reporting.

fe26038

Take execution status into account as well when determining the value for the 'in_motion' field of the Robot Status message.

driver: avoid using uninitialised memory.

5832325

This fixes velocities being reported by the driver for axes that are not part of the motion group.

driver: apply feedback decoupling correctly. Fix #223.

62dde8b

This should only be applied when necessary, and DX200 and YRC1000 already decouple this earlier.

driver: report task creation failure to user.

f9cc4d9

driver: remove v1.8.0 MotoROS binaries.

bb5c2ff

driver: release MotoROS v1.8.1 binaries.

12ab618

EricMarcil mentioned this pull request Jun 26, 2018

Implications of high Command - Feedback latency #219

Closed

1 task

gavanderhoorn commented Jun 26, 2018

View reviewed changes

EricMarcil approved these changes Jun 28, 2018

View reviewed changes

gavanderhoorn merged commit 7de2511 into kinetic-devel Jun 28, 2018

gavanderhoorn deleted the motoros181 branch June 28, 2018 14:21

gavanderhoorn mentioned this pull request Aug 9, 2018

driver: suspicuous magic nr (3) in JointTrajectoryAction::withinGoalConstraints(..) #103

Closed

gavanderhoorn mentioned this pull request Sep 21, 2018

Controller is hanging before reaching trajectory goal #245

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MotoROS v1.8.1 #227

MotoROS v1.8.1 #227

gavanderhoorn commented Jun 26, 2018

gavanderhoorn commented Jun 26, 2018

gavanderhoorn commented Jun 26, 2018

gavanderhoorn left a comment

gavanderhoorn Jun 26, 2018

EricMarcil Jun 26, 2018

gavanderhoorn Jun 26, 2018

EricMarcil Jun 26, 2018

gavanderhoorn Jun 26, 2018

gavanderhoorn Jun 26, 2018

gavanderhoorn Jun 26, 2018

EricMarcil Jun 26, 2018

gavanderhoorn Jun 26, 2018

gavanderhoorn Jun 26, 2018

EricMarcil Jun 26, 2018

gavanderhoorn commented Jun 26, 2018

gavanderhoorn commented Jun 26, 2018

gavanderhoorn commented Jun 26, 2018

andreaskoepf commented Jun 28, 2018

gavanderhoorn commented Jun 28, 2018

gavanderhoorn commented Jun 28, 2018 •

edited

Loading

gavanderhoorn commented Jun 28, 2018

gavanderhoorn commented Jun 28, 2018

MotoROS v1.8.1 #227

MotoROS v1.8.1 #227

Conversation

gavanderhoorn commented Jun 26, 2018

gavanderhoorn commented Jun 26, 2018

gavanderhoorn commented Jun 26, 2018

gavanderhoorn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gavanderhoorn commented Jun 26, 2018

gavanderhoorn commented Jun 26, 2018

gavanderhoorn commented Jun 26, 2018

andreaskoepf commented Jun 28, 2018

gavanderhoorn commented Jun 28, 2018

gavanderhoorn commented Jun 28, 2018 • edited Loading

gavanderhoorn commented Jun 28, 2018

gavanderhoorn commented Jun 28, 2018

gavanderhoorn commented Jun 28, 2018 •

edited

Loading