search

Yaskawa-Global / motoros2

ROS 2 (rcl, rclc & micro-ROS) node for MotoPlus-compatible Yaskawa Motoman robot controllers
96 stars 20 forks source link

Raise alarm if calib data can't be loaded on multi-group systems with TF enabled #169

Closed gavanderhoorn closed 7 months ago

gavanderhoorn commented 1 year ago

But only if it hasn't been explicitly disabled by the user.

Should fix #54.

For the discussion: see also #54.

Haven't tested it yet.

gavanderhoorn commented 1 year ago

Note: I intentionally didn't update subcode documentation as I'd like to test the new subcode doc linter.

Edit: added the documentation. Linter is happy again.

gavanderhoorn commented 1 year ago

Tested, works:

Ros_Controller_Initialize: calib loaded ok: 0, should warn: 1
Ros_Controller_Initialize: no calibration files loaded: TF potentially incorrect -> warn (disable with 'disable_missing_calib_warning')

on a YRC1 with a GP25 on a track without calibration.

ted-miller commented 1 year ago

on a YRC1 with a GP25 on a track without calibration

This comment got me thinking. I think I went about the base-track-tf all wrong. I don't think that base tracks use the same calibration method as multi-robot and station-axes.

Do you even have a Robot Calib or Grp Combination button on your pendant?

image

image

I think the base track uses different parameters for its calibration. It's basically an implied group combination.

If you jog your track, does your /tf visualization move with it?

Assuming I'm right about the base calibration (or lack thereof), then this alarm will always occur on a system with a base. Also, it would mean that /tf is never going to work right.

gavanderhoorn commented 1 year ago

Do you even have a Robot Calib or Grp Combination button on your pendant?

nope.

If you jog your track, does your /tf visualization move with it?

nope.

Also, it would mean that /tf is never going to work right.

with the current implementation I guess? What's needed would be to broadcast the current Yaskawa Base to Yaskawa RF IIUC? Or at least add it to the world -> base (non-Yaskawa base that is) transform we're broadcasting now.

gavanderhoorn commented 1 year ago

Opened #170 to track the TF-is-incorrect-for-robots-on-tracks issue.

ted-miller commented 1 year ago

Also, it would mean that /tf is never going to work right.

with the current implementation I guess?

Yeah.

What's needed would be to broadcast the current Yaskawa Base to Yaskawa RF IIUC?

The calibration from BF to RF exists. It's just not in the mpGetCalibration data. I think it's in SCG parameters.

gavanderhoorn commented 11 months ago

I believe this should now work.

In essence, this checks whether there are groups which don't have an associated base group and if there are, and if no calibration files loaded, TF is enabled and the warning is not suppressed, it raises the alarm.

In all other cases it stays silent.

On my configuration this prints:

Ros_Controller_Initialize: calib loaded ok: no, should warn: no
gavanderhoorn commented 11 months ago

@ted-miller: could you perhaps test this on some more complex group configurations?

ted-miller commented 11 months ago

Sure. It'll be Monday or Tuesday. I've currently got everything configured for single-robot.

gavanderhoorn commented 11 months ago

Friendly ping @ted-miller.

ted-miller commented 11 months ago

So, with R1+R2+S1, if I don't have any calibrations, I get the alarm. If I then add a cal for R2+S1 and reboot, there is no alarm.

gavanderhoorn commented 10 months ago

@ted-miller: I've updated the PR with the changes from the branch I mentioned in https://github.com/Yaskawa-Global/motoros2/pull/169#discussion_r1384674728. I've also included the changes to address your review comments from #189.

gavanderhoorn commented 8 months ago

@ted-miller: I believe this addresses your latest feedback.

I've tested it on my controller, and it appears to work -- as in: I don't get an alarm with just my base axis, even though I have not disabled the alarm nor have I any calibration file(s).

Could you perhaps try this on your setup?

ted-miller commented 8 months ago

My multi-group setup is being used for testing something else. It should free up later this week.

gavanderhoorn commented 8 months ago

My multi-group setup is being used for testing something else. It should free up later this week.

you guys do other things beside maintaining/developing MotoROS2?

PS: running it on a single-group system would also be a good test.

ted-miller commented 8 months ago

you guys do other things beside maintaining/developing MotoROS2?

Of course. We're scoping out the framework for MotoROS3. What else would we be doing?

gavanderhoorn commented 7 months ago

Friendly ping @ted-miller :)

ted-miller commented 7 months ago

Still on the todo list. Haven't forgotten this.

gavanderhoorn commented 7 months ago

Friendly ping @ted-miller :)

ted-miller commented 7 months ago

Still on the todo list. Haven't forgotten this.

Ok... I did forget this. I won't get to it today. But I should be able to this week.

gavanderhoorn commented 7 months ago

I don't really have time to figure out why the tests crash the controller. I have a hunch it's stack related, but haven't been able to check it.

I would be OK with merging without the tests, as I have been able to run them by disabling the other tests. They passed.

As it's been a while, I'd like to rebase this and then merge.

gavanderhoorn commented 7 months ago

Oh, right. CI is broken (https://github.com/Yaskawa-Global/motoros2/pull/212#issuecomment-1963952062).

Well, it builds, I promise :crossed_fingers:

@ted-miller: ok with the merge?