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

Service and message definitions for reading and writing variables #24

Draft
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

yeu-geissdoerfer
Copy link

Added service definitions for reading and writing each variable type. A message definition for the error codes has also been added.

Each service definition has "var_number" as the variable name in the request section, as this wording fits better with the MotoPlus function definition used, than using the name "address".

For reading and writing position variables, the service has been kept as proposed. The positions are in the ROS-Industrial base frame. A corresponding motoros2 function will convert this ROS-Industrial base frame position to a Yaskawa base frame position and vice versa, since the robot controller it self cannot deal with the ROS-Industrial base frame directly. If needed, the option to define positions in the tool or user frame can be added in a later version.

yeu-geissdoerfer and others added 16 commits September 9, 2024 09:01
Use var_number, in the request section, instead of address as MotoPlus uses the term var_no in their documentation.
Use var_number, in the request section, instead of address as MotoPlus uses the term var_no in their documentation.
Use var_number, in the request section, instead of address as MotoPlus uses the term var_no in their documentation.
Use var_number, in the request section, instead of address as MotoPlus uses the term var_no in their documentation.
Use var_number, in the request section, instead of address as MotoPlus uses the term var_no in their documentation.
Use var_number, in the request section, instead of address as MotoPlus uses the term var_no in their documentation.
Use var_number, in the request section, instead of address as MotoPlus uses the term var_no in their documentation.
Use var_number, in the request section, instead of address as MotoPlus uses the term var_no in their documentation.
Use var_number, in the request section, instead of address as MotoPlus uses the term var_no in their documentation.
Use var_number, in the request section, instead of address as MotoPlus uses the term var_no in their documentation.
Use var_number, in the request section, instead of address as MotoPlus uses the term var_no in their documentation.
Use var_number, in the request section, instead of address as MotoPlus uses the term var_no in their documentation.
@gavanderhoorn
Copy link
Collaborator

Each service definition has "var_number" as the variable name in the request section, as this wording fits better with the MotoPlus function definition used, than using the name "address".

could I suggest index instead of var_number?

The former matches the M+ API spec even closer:

typedef struct
{
  USHORT usType;
  USHORT usIndex;
} MP_VAR_INFO;

and is also less ambiguous ('number' could also mean 'amount').

@yeu-geissdoerfer
Copy link
Author

yeu-geissdoerfer commented Sep 12, 2024

I am using the M+ function mpPutUserVars for writing variables and mpGetUserVars for reading variables, as these functions can read and write user variables in high speed.

typedef struct
{
    int var_type;
    int var_no;
    union {
        MP_B_VAR_BUFF b;
        MP_I_VAR_BUFF i;
        MP_D_VAR_BUFF d;
        MP_R_VAR_BUFF r;
        MP_S_VAR_BUFF s;
        MP_P_VAR_BUFF p;
        MP_P_VAR_BUFF bp;
        MP_P_VAR_BUFF ex;
    } val;
} MP_USR_VAR_INFO;

Here they use int var_no. That is why I suggested to use var_number as I think number is easier to read than no.

Would be var_no better than var_number?

@gavanderhoorn
Copy link
Collaborator

gavanderhoorn commented Sep 12, 2024

I know the manual states it's much faster, but I'm wondering if that is even noticeable compared to the ROS 2 service overhead.

Even with this in mind, I might still suggest using index. But I won't insist on it.

@ted-miller
Copy link
Collaborator

Personally, I'd suggest actually using address to be consistent with the I/O services. The MotoROS layer obfuscates over the Motoplus API.

msg/VarResultCodes.msg Outdated Show resolved Hide resolved
@gavanderhoorn
Copy link
Collaborator

High-level comment: would we want to 'condense' things a bit? By which I mean: instead of having separate services for reading and writing bytes and ints, we could just have one, which truncates whatever comes in if it's writing to a byte variable.

Same for floats (reals) and doubles.

I'm not a fan of union message definitions, so that's not what I'm suggesting, but right now we'd be adding 12 services to MotoROS2. The alternative (after combining a few) would save us 4 service servers.

This would be different from the IO ROS API though, where we didn't do this, but I wanted to put it up for discussion at least.

@ted-miller
Copy link
Collaborator

instead of having separate services for reading and writing bytes and ints, we could just have one
Same for floats (reals) and doubles

In this case, double is a double-width integer (DINT). So you'd have one service for BID, one for R, and one for S.

IMO, they should all be separate services. But I don't feel strongly either way. My only concern is whether the extra services impacts performance of the executor that monitors them.

@gavanderhoorn
Copy link
Collaborator

gavanderhoorn commented Sep 12, 2024

In this case, double is a double-width integer (DINT). So you'd have one service for BID, one for R, and one for S.

ah, right. I confused it with double precision floating point.

My only concern is whether the extra services impacts performance of the executor that monitors them.

should not matter too much.

Either we add a new executor, or we add it all to the IO executor.

I don't expect performance issues, as long as clients don't start calling these services at high frequencies. Which is not what services are for.

It's just another 14 entries in the ros2 service list output.

@yeu-geissdoerfer
Copy link
Author

Either we add a new executor, or we add it all to the IO executor.

In the MR2 implementation that I have started, I have already added a new executor.

I don't expect performance issues, as long as clients don't start calling these services at high frequencies. Which is not what services are for.

I agree on that.

@gavanderhoorn
Copy link
Collaborator

Either we add a new executor, or we add it all to the IO executor.

In the MR2 implementation that I have started, I have already added a new executor.

yes, I saw (Yaskawa-Global/motoros2@3e66e15).

But we should avoid starting another task for it. That should not be needed.

Just add it to the task that's spinning the IO executor.

@yeu-geissdoerfer
Copy link
Author

Another question would be whether we really need to expose all variables types?
Do we really need byte, int, double and real all together. Wouldn't it be enough to have byte and real?
But on the other hand side we should expose all functionalities.

@gavanderhoorn
Copy link
Collaborator

Do we really need byte, int, double and real all together. Wouldn't it be enough to have byte and real?

my initial response would indeed be: yes, we should offer all of them.

Can you motivate why we could leave out int and double?

That would make it impossible to write numbers larger than 255 to variables and make the I and D variables unavailable.

@yeu-geissdoerfer
Copy link
Author

I saw it more from the application side, how I would use the services.

  • Position variables make sense to me so that you can determine a goal position externaly, writing the position variable and then the INFORM job drives to the position without the need of external path planning.
  • String variables make sense for writing messages to interact with a pendant user for example.
  • Real variables would offer larg floating point values.
  • Byte variables would make sense for counting up a number.

But this would be only a thing if we want to reduce the amount of services, as we mentioned earlier.

Copy link
Collaborator

@ted-miller ted-miller left a comment

Choose a reason for hiding this comment

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

[External email]: I have a question regarding transforming the Pose from Ros to Yaskawa base frame.

I've got some conversion examples in

https://github.com/Yaskawa-Global/motoros2/blob/c61fb86e2cf31ff894189efd8a36a2f1fc5d1161/src/PositionMonitor.c#L253

This function specifically converts from a Yaskawa cartesian position to a Transform

https://github.com/Yaskawa-Global/motoros2/blob/c61fb86e2cf31ff894189efd8a36a2f1fc5d1161/src/RosMotoPlusConversionUtils.c#L20

(I'm not sure if this answers your original question or not

srv/WriteVarPosition.srv Show resolved Hide resolved
srv/ReadVarPosition.srv Show resolved Hide resolved
@yeu-geissdoerfer
Copy link
Author

So, how should we implement this now?

We also need to make a decision regarding naming the variable number address or index.

@ted-miller
Copy link
Collaborator

So, how should we implement this now?

I think we should allow access to all variable types (BIDRSPBpEx).

We also need to make a decision regarding naming the variable number address or index

I vote for address. But if you're doing the implementation, then I think you get to ruling vote.

@yeu-geissdoerfer
Copy link
Author

Okay, then lets take ´address´.

So until now I have allowed access to the variable types (BIDRS) and for P I need to implement it. But here the question remains regarding the position representation.

After finishing this implementation I would implement Bp and Ex in a step two.

@ted-miller
Copy link
Collaborator

But here the question remains regarding the position representation.

geometry_msgs/PoseStamped in ros-base/yaskawa-RF makes sense for the majority of applications.

But I do think it makes sense to allow for additional formats. So perhaps a struct of:

geometry_msgs/PoseStamped   // base pose
sensor_msgs/JointState              // robot joints
geometry_msgs/Transform         // offset or IMOV

Though I'm not sure how you indicate which of these struct members are 'valid'. But perhaps that's up the developer of the end-application?

@yeu-geissdoerfer
Copy link
Author

okay that sounds good!

@gavanderhoorn
Copy link
Collaborator

@yeu-geissdoerfer: friendly ping?

@yeu-geissdoerfer
Copy link
Author

@gavanderhoorn Unfortunately I don't have time to finish the implementation at the moment.
My status is that every variable type except P-variables can be read and written. I have already tested this successfully in our lab.

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.

3 participants