Yaskawa-Global / motoros2

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

Add support for INFORM job CRUD #278

Open gavanderhoorn opened 1 month ago

gavanderhoorn commented 1 month ago

From the roadmap:

The following items are on the MotoROS2 roadmap, and are listed here in no particular order:

  • [..]
  • CRUD of INFORM job files (ie: create, retrieve, update, delete)
  • [..]

In addition to the listed functionality, we'd also need to be able to list existing jobs, as the other operations will need names of jobs to operate on.

The goal of such services would be to allow a ROS 2 application to completely manage robot native INFORM jobs. Coupled with INFORM job coordination services, this would allow a ROS 2 application to use MotoROS2 to create, retrieve, update, delete, start and stop INFORM jobs.

ROS-based motion planning could be used to bring a robot (/EEF) to a specific location where an INFORM job could take over and perform some specific process (such as welding, using the Yaskawa provided support for welding).

In summary: it would make hybrid ROS 2 + INFORM applications possible.

Progress:

gavanderhoorn commented 1 month ago

Current design of the services: here (diff).

gavanderhoorn commented 3 weeks ago

Current WIP of everything except inform_job/list: https://github.com/Yaskawa-Global/motoros2/compare/inform_job_crud_list...gavanderhoorn:motoros2:inform_job_crud_the_rest.

Service definitions: https://github.com/Yaskawa-Global/motoros2_interfaces/compare/main...gavanderhoorn:motoros2_interfaces:inform_crud.

gavanderhoorn commented 3 weeks ago

I've been trying to test my latest changes (adding 'the rest' of the CRUD operations), but for some reason none of the new services 'work'. The client just hangs. No output on the server side (in the debug log).

Almost as-if the executor isn't processing the events. Removing the list service (ie: one that definitely works) from the executor results in the same behaviour (of that service).

I've compared the code (diff) to other service additions, but can't really find anything incorrect.

I'm probably missing something. @ted-miller: could you maybe take a look and see whether you can spot anything?

ted-miller commented 3 weeks ago

It seems that the buffer for the incoming name field needs to be allocated in Ros_ServiceGetInformJob_Initialize (and other init functions)

    rosidl_runtime_c__String__init(&g_messages_GetInformJob.request.name);
    rosidl_runtime_c__String__assign(&g_messages_GetInformJob.request.name, "this is a string to allocate a buffer");

I'm not really sure why assign was also needed. But it worked after I added that.

gavanderhoorn commented 3 weeks ago

Ah, right :coffee: :sleepy:


Edit: switched to using micro_ros_utilities instead of doing this all manually: 2bc333f7d2060eb5b3e9da2968e9e0b0f2ed61d8.

gavanderhoorn commented 3 weeks ago

The latest additions now work, but there's one thing to consider/discuss: I initially configured a maximum job file size of 4 KB. That turns out to be way too small, so I increased it to 64 KB.

But, as we need / want to preallocate all resources for all services, this means we're preallocating 128 KB just for the get and put services.

Total memory usage on my YRC1 is now approximately 220 KB.

gavanderhoorn commented 3 weeks ago

@ted-miller: could you perhaps check what a useful maximum upper limit for job files could be? I don't have access to anything representative as I don't normally use INFORM.

ted-miller commented 3 weeks ago

The biggest job I have locally on my PC is 28 KB.

But, as we need / want to preallocate all resources for all services, this means we're preallocating 128 KB just for the get and put services

I'm not certain that we need to do that. In my example above, I use the string "this is a string to allocate a buffer". But the job I requested was much larger than that and the whole thing was transferred. So, it's possible that the executor is reallocating the buffer as needed. (And it's also possible that I just had a massive buffer overflow and overwrote a bunch of memory inadvertently.)

gavanderhoorn commented 6 hours ago

So technically you're correct. We don't have to pre-allocate. However, the micro_ros_utilities(..) functions I now use (2bc333f7d2060eb5b3e9da2968e9e0b0f2ed61d8) do pre-allocate, and they will only do that based on the size configured for those fields.

We would have to revert to manually managing the request and reply fields, but:

But the job I requested was much larger than that and the whole thing was transferred. So, it's possible that the executor is reallocating the buffer as needed. (And it's also possible that I just had a massive buffer overflow and overwrote a bunch of memory inadvertently.)

I would expect the latter to have happened.

I don't believe assign(..) checks anything (that would be assignn(..)'s job).