search

utexas-bwi / rocon_scheduler_requests

Interfaces for managing rocon scheduler requests
http://wiki.ros.org/rocon_scheduler_requests
0 stars 3 forks source link

What triggers the replies field in a request set? #13

Closed stonier closed 10 years ago

stonier commented 10 years ago

I'm seeing this below in a request set feedback response:

requester_id: e85001eb-6b30-42cf-9c17-a2b8175a7bf1
replies: False
requests:
  id: 5ff20c1c-1c78-43bf-b9fc-fddd9a9302f4
    priority: 0
    resources: 
      linux.*.ros.pc.dudette/rocon_apps/talker
      linux.*.ros.pc.dude/rocon_apps/listener
      linux.*.ros.pc.dude1/rocon_apps/listener
    status: 2
jack-oquin commented 10 years ago

It needs better documentation.

It's a parameter of the RequestSet() constructor that determines whether it contains ResourceRequest or ResourceReply elements. Those, in turn, limit what operations can be done with the requests to specifically requester or scheduler transitions, respectively. The replies field is intentionally ignored when testing request sets for equality.

The requester module sends request sets with replies=False and tags received feedback with replies=True. The scheduler module receives requests with replies=False and sends feedback with replies=True. Now I've written that, it appears unnecessarily convoluted. Probably each side of the protocol can exclusively use its own type for both incoming and outgoing messages. That should work and will be easier to explain and understand. I'll look into making that change.

It's all a hack. I am open to suggestions for a cleaner solution. The obvious alternative is making a pair of derived classes based on RequestSet. So far, I've been reluctant to do it that way.

jack-oquin commented 10 years ago

I'll probably make the change described above. Using the types consistently within the scheduler node and the requester nodes is much better.

With that simplification, defining two different derived classes makes sense. Those classes could be defined in the scheduler and requester modules, so it would always be clear which to use. Then the replies field would not be needed any more.

That looks like a good solution, but I am open to other ideas.

The replies field was added when I introduced ResourceRequest and ResourceReply as separate classes with different state transition methods. That was a good idea, but the replies hack was not well thought-out.

jack-oquin commented 10 years ago

The commit above removes the replies parameter and data member, replacing it with a contents parameter, which should either be ResourceRequest (the default) or ResourceReply (when instantiated within a scheduler).

With that change, I don't think RequestSet needs any derived classes, but I am thinking of renaming ResourceReply to something else, maybe ResourceResponse.

stonier commented 10 years ago

Just caught up on this - it looks good jack.