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

Support for FS100 controllers #227

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open

Support for FS100 controllers #227

wants to merge 20 commits into from

Conversation

gavanderhoorn
Copy link
Collaborator

This is for #19 (and a resubmit of #226 from a branch here on the main repository).

A draft currently, but first steps towards FS100 support.

@gavanderhoorn
Copy link
Collaborator Author

gavanderhoorn commented Mar 15, 2024

CI will fail as we currently have no release of M+ libmicroros for FS100.

We'll deal with that later.


Edit: and of course because of #212 (comment).

@gavanderhoorn

This comment was marked as outdated.

@gavanderhoorn
Copy link
Collaborator Author

Something to decide: the current work-around/hack mentioned in #19 (comment) only works for Agents running on little-endian machines.

I expect that to be the typical setup for use with MotoROS2 (ie: amd64 based ROS PC running the Agent), but I could see someone trying to use an RPi or similar aarch64 or aarch32 platform to delegate the Agent to.

That would not work for the FS100.

Personally, I feel we could document this, but:

  1. the asymmetry with the other controllers is not nice (™)
  2. MotoROS1 had no such limitation
  3. it's likely such a note in our documentation will be overlooked, leading to users posting issues and/or opening discussions about their installation not working

Given the fact the FS100 is a legacy platform at this point this may all be acceptable, but we need to decide how we want to approach this.

@ted-miller
Copy link
Collaborator

There's only AUTO, CYCLE and STEP. If I set it to AUTO, SOUT#035 becomes high, but as soon as I switch the TP to Remote, cycle gets reset to CYCLE.
Any ideas?

Go to Setup > Operate Cond. Then change the CYCLE SWITCH IN REMOTE MODE setting.

Given the fact the FS100 is a legacy platform at this point this may all be acceptable, but we need to decide how we want to approach this

I'm of this opinion. Just document the limitation and move on.

There will be a limited set of users using MR2 on FS100 and a much smaller set of those using a bigendian cpu.

@ted-miller
Copy link
Collaborator

ted-miller commented Mar 25, 2024

@gavanderhoorn Beyond the potential endianness issue, what concerns do you have? Given the success of the previous testing, I'm not really sure why this is a 'draft' PR.

My biggest concern is that we haven't validated on a non-dummy controller. I'm going to try to scrounge up one of the old educational carts.

Now that the rebase is complete, I'm happy to test all topics/services again. Please provide a binary of the libmicroros. (I'll also validate the VS settings for building MR2.)

@ted-miller
Copy link
Collaborator

I've located an FS100 with an arm. It should be here tomorrow afternoon.

@gavanderhoorn
Copy link
Collaborator Author

It's partly a draft because I haven't actually moved anything, neither real hw nor virtual.

It also lacks documentation updates, and I still need to verify which parameters need to be set to which values.

@ted-miller
Copy link
Collaborator

ted-miller commented Mar 25, 2024

I still need to verify which parameters need to be set to which values.

From my personal notes regarding the FS100:

I don't think that S2C1402 exists (EDIT) is used.

Everything else should be covered by our existing setup-instructions.

@ted-miller
Copy link
Collaborator

ted-miller commented Mar 27, 2024

@gavanderhoorn
Can you provide the latest libmicroros or .out?
Or would you prefer I validate using <outdated_binary> ?

@gavanderhoorn
Copy link
Collaborator Author

gavanderhoorn commented Mar 28, 2024

Or would you prefer I validate using <outdated_binary> ?

that wouldn't work, as it has DUMMY_SERVO_MODE enabled.

I'll build a fresh alpha.

@gavanderhoorn
Copy link
Collaborator Author

gavanderhoorn commented Mar 28, 2024

@ted-miller: here you go: <outdated_binary>.

@gavanderhoorn gavanderhoorn marked this pull request as ready for review March 28, 2024 14:33
@ted-miller
Copy link
Collaborator

I'll do some more thorough testing next week. But doing a quick test, I was able to get the robot to move with the point-queue service.

Other quick notes:

  • effort data in joint_states is garbage data
  • velocity fields in joint_states are always zero
  • The feedback position is 'laggy'. If I jog the robot, the position immediately updates. But when I release the jogging button, the position coasts for about a second before coming to a stop. I see this same behavior on the pendant, so it's not MR2. But it is something different with the FS100.

@gavanderhoorn
Copy link
Collaborator Author

I'll do some more thorough testing next week. But doing a quick test, I was able to get the robot to move with the point-queue service.

👍

Other quick notes:

  • effort data in joint_states is garbage data

hm. Is this a known issue? I'm running FS4.00 here.

  • velocity fields in joint_states are always zero

Probably due to the M-register for velocity feedback not being supported on FS100? At least that's what logged on my controller when it boots up MotoROS2.

  • The feedback position is 'laggy'. If I jog the robot, the position immediately updates. But when I release the jogging button, the position coasts for about a second before coming to a stop. I see this same behavior on the pendant, so it's not MR2. But it is something different with the FS100.

I believe I've seen something similar, but wasn't sure whether it was caused by the fact I don't have any manipulator connected.

@ted-miller
Copy link
Collaborator

effort data in joint_states is garbage data

hm. Is this a known issue? I'm running FS4.00 here.

No. The M+ manual says the API should work. But I looked at MR1 code and see that Ros_CtrlGroup_GetTorque never actually gets called anywhere. So, I don't have any known examples of the API working on the FS100.

velocity fields in joint_states are always zero

Probably due to the M-register for velocity feedback not being supported on FS100? At least that's what logged on my controller when it boots up MotoROS2.

Yup. That'll do it.

image

I guess we need to convert Ros_CtrlGroup_GetFBServoSpeed to use mpSvsGetVelTrqFb. But that's assuming mpSvsGetVelTrqFb actually works... (It's what is being used for effort data.)

@gavanderhoorn
Copy link
Collaborator Author

I guess we need to convert Ros_CtrlGroup_GetFBServoSpeed to use mpSvsGetVelTrqFb. But that's assuming mpSvsGetVelTrqFb actually works... (It's what is being used for effort data.)

I believe we have an open issue on the internal tracker (#547) where in the end I commented:

According to yourself (here), M-registers are better ;)

I was confused by the comments in that issue as you also suggested changing to mpSvsGetVelTrqFb(..), but that was/is already used. At least in MotoROS1.

@ted-miller
Copy link
Collaborator

(Not sure if this note is a repost. I know I had something typed up, but I think I closed the window prematurely.)

I made a small app that just polls mpSvsGetVelTrqFb continuously.

For velocity:
With servos off and the brakes engaged, I'm getting random +/- 1248 on one or more axes every few cycles. But while jogging, the numbers look reasonable.

For effort:
I don't know what to think of the numbers being displayed. I need to do more testing.
The pendant only displays percent of rated max torque, so I don't have a good correlation between this and Nm.

But it seems like there's a lot of fluctuation on multiple axes for even a very simple motion, such as jogging the S axis. Also, the resting-position seems unusual. If I jog S+ the value spikes. When motion is complete, the value settles around 15000. But then if I jog S- and stop, the value settles around -10000. I understand the sign inversion, but I don't get why the absolute value is significantly different.


I tested a motion with the FJT script. The motion worked, but the final position was outside of tolerance on both axes that I moved by 0.012 rad.

I'm wondering if this is related to the 'laggy' response I get when jogging.


In the error message about the trajectory being outside tolerance, the joint names were non-ascii characters.

snprintf(formatBuffer, 64, " [%s: %.4f deviation]",
feedback_FollowJointTrajectory.feedback.joint_names.data[axis],
violators[axis]);

This has me questioning if something is overflowing an array. Or perhaps this isn't working properly.

@gavanderhoorn
Copy link
Collaborator Author

Hm. Sounds like something isn't working.

re: velocities & torques: could this be rate-dependent?

@gavanderhoorn
Copy link
Collaborator Author

gavanderhoorn commented Apr 16, 2024

@ted-miller: I've added dd13f53 which (im)ports the MotoROS1 version of Ros_CtrlGroup_GetFBServoSpeed(..) to use mpSvsGetVelTrqFb(..) instead of the M-registers.

@gavanderhoorn gavanderhoorn force-pushed the fs100_support branch 2 times, most recently from dd13f53 to d4fb2fd Compare April 16, 2024 10:09
@gavanderhoorn
Copy link
Collaborator Author

@ted-miller: I just remembered I haven't actually built this branch after I added d4fb2fd.

@gavanderhoorn
Copy link
Collaborator Author

@ted-miller wrote:

In the error message about the trajectory being outside tolerance, the joint names were non-ascii characters.

snprintf(formatBuffer, 64, " [%s: %.4f deviation]",
feedback_FollowJointTrajectory.feedback.joint_names.data[axis],
violators[axis]);

Should that be feedback_FollowJointTrajectory.feedback.joint_names.data[axis].data? Technically feedback_FollowJointTrajectory.feedback.joint_names.data[axis] is a rosidl_runtime_c__String, not a const char*.

This has me questioning if something is overflowing an array. Or perhaps this isn't working properly.

Technically I believe we could use rosidl_runtime_c__String__copy(..) instead of 'abusing' _assign(..), but it wouldn't really make much difference IIUC.

@ted-miller
Copy link
Collaborator

I just remembered I haven't actually built this branch after I added d4fb2fd.

I was able to build this yesterday. Though, I did have to shrink SIZEOF_BUFFER_FJT_GOAL yet again to make it load without 1020 [6]. (Admittedly, I didn't strip the binary after building and it was 300-400k larger than alpha50. So perhaps that would have made a difference.)

Also, there were a bunch of annoying warnings about an unneeded const identifier. I'm thinking that we should remove the -Wall from the compiler arguments for FS100. Presumably, any "important" warnings would also be raised by the DX/YRC compilers.

Should that be feedback_FollowJointTrajectory.feedback.joint_names.data[axis].data

Technically, yes. But since the string pointer is the first entry in that structure, it should be ok.

@gavanderhoorn
Copy link
Collaborator Author

gavanderhoorn commented Apr 25, 2024

Though, I did have to shrink SIZEOF_BUFFER_FJT_GOAL yet again to make it load without 1020 [6]. (Admittedly, I didn't strip the binary after building and it was 300-400k larger than alpha50. So perhaps that would have made a difference.)

that should indeed not really be needed.

I may be doing something 'different' when building it here. I'll take a look at my settings.

Also, there were a bunch of annoying warnings about an unneeded const identifier. I'm thinking that we should remove the -Wall from the compiler arguments for FS100. Presumably, any "important" warnings would also be raised by the DX/YRC compilers.

I don't remember anything about const modifier (I guess?), I do have warnings about return types, for which I added -Wno-return-type. But only for the FS100 builds.

Should that be feedback_FollowJointTrajectory.feedback.joint_names.data[axis].data

Technically, yes. But since the string pointer is the first entry in that structure, it should be ok.

technically, yes.

But after I changed that when working on #233, the issues with doubles and Ros_Debug_BroadcastMsg(..) stopped.

@gavanderhoorn
Copy link
Collaborator Author

@ted-miller: have you had any time to take a further look at this?

@ted-miller
Copy link
Collaborator

No I haven't.
I'm definitely not going to be able to get to it until at least the end of the month.

@Sanchitkedia
Copy link

@gavanderhoorn I have a Yaskawa Motoman SIA20F with an FS100 controller and can try to run this on my end. Could you help me with the necessary files and instructions on how to get started?

Humble only for now.
Update use of constants and add conditionals to account for this.
So update relevant constants to account for this.
So introduce a custom implementation for FS100 and call that instead.
Gate it on controller platforms which are known to support it (similar
to how we do this in MotoROS1).
Limit use to controller series known to have it and have FS100 just fail
the function completely.
`mpFstat(..)` doesn't appear to work reliably, or at least not for files
on USB sticks and SRAM drive (not sure about main FS).

Use (a custom implementation of) `tell(..)` to determine actual file
size and use it to override `stat::st_size`.
Previous conditional was too conservative. Add FS100 to supported controllers.
Otherwise the binary won't load, and this should still be sufficient for the maximum size of the goals we want/need to support.
It doesn't support the M-register approach, so make use of mpSvsGetvelTrqFb(..) instead.
Remove all other controllers + M+ `libmicroros` tuples.
@gavanderhoorn
Copy link
Collaborator Author

@jfernandez37: I started work to try and reproduce what you've reported last week, but haven't been able so far. My FS100 isn't setup with physical hw, which may be why. I'll try some more ways to 'break' it.

@gavanderhoorn
Copy link
Collaborator Author

@jfernandez37: I'm starting to suspect the sleep/timer implementation on FS100 platforms (not the base OS, but MotoROS2's wrapper/use of it), but haven't been able to pin-point anything yet.

Have you observed any more issues in the meantime?

And thank you for testing the alpha. This is valuable.

@jfernandez37
Copy link

No, we haven't noticed any more issues with it. Thank you for your help.

@gavanderhoorn
Copy link
Collaborator Author

#296 fixes at least one case of "strange chars" in the debug log of the FS100 build I'm testing.

@gavanderhoorn
Copy link
Collaborator Author

Just to provide an update: @jfernandez37: due to the way my FS100 is configured I can't seem to reproduce the problem(s) you encountered.

I've tried to break things by executing many different trajectories all over the (combined) joint position ranges of all joints without running into any issues so far. I've identified a couple of other issues with the port which we'll be addressing later, but we're going to need a different setup to debug.

I believe @ted-miller may have an opportunity soon to assist.

@ted-miller
Copy link
Collaborator

I have not been able to reproduce this issue either. Using a small 6-axis arm, I'm able to execute any trajectories repeatedly.

I also tried reconfiguring as a 7-axis SIA (with a virtual arm). Still not able to cause this issue.

It seems the problem is with the micro-ros agent. Please ensure that the latest humble docker image is being used.


Other oddities:

  • I didn't initially follow the setup instructions properly and forgot to set some of the parameters. I got fatal alarm [10] (couldn't create publisher) until I set these and rebooted.
    I don't understand why this alarm occurred.

  • When setup as a virtual arm, I got a whole lot of Failed to get pulse position (command). But the next day, that went away. I don't think I changed anything. But I can't explain this. (Probably a moot point, since I was configured for DUMMY mode.)

  • start_traj_mode didn't work in DUMMY mode either. It wouldn't enable the servos automatically. I had to manually turn them on and then switch to REMOTE mode. (Again, moot since this is DUMMY)

@gavanderhoorn
Copy link
Collaborator Author

gavanderhoorn commented Sep 6, 2024

@ted-miller wrote:

I have not been able to reproduce this issue either. Using a small 6-axis arm, I'm able to execute any trajectories repeatedly.

did you encounter any other issues while executing those trajectories?

This was with actual HW right?

I also tried reconfiguring as a 7-axis SIA (with a virtual arm). Still not able to cause this issue.

It seems the problem is with the micro-ros agent. Please ensure that the latest humble docker image is being used.

Just making sure (again? perhaps) @jfernandez37: you are running this on an amd64 desktop/laptop and are indeed using the latest Agent Docker image?

Could (probably is) something else, but just to rule things out.

One possible other cause of the error mentioned in #227 (comment) would be a buffer overflow (on the MR2 side) and/or memory corruption.

Other oddities:

  • I didn't initially follow the setup instructions properly and forgot to set some of the parameters. I got fatal alarm [10] (couldn't create publisher) until I set these and rebooted.
    I don't understand why this alarm occurred.

Yes, I remember running into this myself. I'm also unsure what causes that. I've considered it a red herring for now.

  • When setup as a virtual arm, I got a whole lot of Failed to get pulse position (command). But the next day, that went away. I don't think I changed anything. But I can't explain this. (Probably a moot point, since I was configured for DUMMY mode.)

was this with DUMMY_SERVO_MODE on the MR2 side, or without?

  • start_traj_mode didn't work in DUMMY mode either. It wouldn't enable the servos automatically. I had to manually turn them on and then switch to REMOTE mode. (Again, moot since this is DUMMY)

Also ran into this. Had to use the same work-around.

thanks for testing.


At this point I'll see if can create a heavily instrumented version of MotoROS2 which logs just about everything around the trajectory processing, re-ordering and execution queues.

@jfernandez37
Copy link

Yes. We are running the most recent version of the docker image and on an am64 laptop.

@gavanderhoorn
Copy link
Collaborator Author

Apologies, that was supposed to read amd64 (ie: x86_64). But thank you for confirming.

@jfernandez37
Copy link

I just checked the laptop that we have been using, and it has an Intel cpu

@gavanderhoorn
Copy link
Collaborator Author

gavanderhoorn commented Sep 6, 2024

I just checked the laptop that we have been using, and it has an Intel cpu

that's ok. amd64 is an architecture, not a brand. As long as it's not an ARM based processor (such as in an RPi), or something more exotic. The current version of MotoROS2 for FS100 is only compatible with x86_64.

@ted-miller
Copy link
Collaborator

did you encounter any other issues while executing those trajectories?

I got the "outside tolerance" failure that was mentioned previously. I think there's something "laggy" with the FS100.

But the arm executed the motion just fine.

This was with actual HW right?

The 6 axis tests were with a physical arm. The 7 axis tests were dummy-mode.

When setup as a virtual arm, I got a whole lot of Failed to get pulse position (command). But the next day, that went away. I don't think I changed anything. But I can't explain this. (Probably a moot point, since I was configured for DUMMY mode.)

was this with DUMMY_SERVO_MODE on the MR2 side, or without?

Yes, DUMMY SERVO MODE was enabled when compiling MR2. Ros_CtrlGroup_GetPulsePosCmd doesn't look at the DUMMY flag because it's not trying to read feedback position.

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