Compute correct service md5sums and generate action messages #25
Conversation
| rtv.request.fields = aggregate[0].fields; | ||
| rtv.response.constants = aggregate[1].constants; | ||
| rtv.response.fields = aggregate[1].fields; | ||
| rtv.request.md5 = rtv.response.md5 = calculateMD5(rtv, "srv"); |
There was a problem hiding this comment.
I had been planning to make the service md5 accessible outside the request or response objects since its the hash of both of them. So the generated service file would export
{
Request: ServiceRequest,
Response: ServiceResponse,
md5() { // return service md5 sum }
}
That seems like it might be difficult for you to duplicate here though?
There was a problem hiding this comment.
That should be fine. I would prefer having a proper service class as well, rather than using the Message class for everything. But I believe we'll need to coordinate this change, for our two class definitions in order to be able to also update where the md5 is being used, such as in ServiceClient: this._messageHandler.Response.md5sum() would need to change to this._messageHandler.md5sum(). Right?
|
All the new action parsing stuff looks good. I'm mostly wondering about how its exposed - with catkin, action message definitions are just turned into message files that, from the client library's perspective, are indistinguishable from regular messages. Importing the generated files from a compared with importing a standard message It's nice that they're exposed separately here, but I'm not sure that I have control over how that gets done in |
|
I see what you are saying. Let me refactor a little. |
|
BTW, how come you guys have not yet run into this issue: Have you been able to control turtlesim (via /turtle1/cmd_vel) using rosnodejs? I was unable to do that before fixing the above in the xmlrpc library. The turtle simply wouldn't move: the tcpros message was never sent, because turtlesim never reacted to the method response of the requestTopic method call. That was because that method response didn't send a content-length header. Have you seen any such issues before? With this out of the way, I should be able to fix up my action handling rather quickly (I'll basically just remove it, since users can use the action messages directly). |
chfritz
force-pushed
the
compute_correct_service_md5sums
branch
2 times, most recently
from
2da36d4 to
736f8fc
Compare
|
OK, refactored and rebased. This should be ready now. I've also added a new example that tests with turtlesim. To run, just run: rosrun turtlesim turtlesim_node
rosrun turtle_actionlib shape_server
node example_turtle.js |
- Fixes RethinkRobotics-opensource#24. - Replaces PR RethinkRobotics-opensource#22. - Refactored the file parsing code a bit in the process; a little cleaner now. Testing use of actionlib topic with turtlesim - now using a patched version of xmlrpc that sends content-length headers on method responses. This omission was preventing tutlesim from receiving publications from rosnodejs, and hence also action lib calls. Simplified construction of complex messages - can now provide all data, incl. for sub-messages, directly in message constructor. E.g.: ```js let now = Date.now(); let secs = parseInt(now/1000); let nsecs = (now % 1000) * 1000; let shapeMsg = new shapeActionGoal({ header: { seq: 0, stamp: { secs: secs, nsecs: nsecs }, frame_id: '' }, goal_id: { stamp: { secs: secs, nsecs: nsecs }, id: "/my_node-1-"+secs+"."+nsecs+"000" }, goal: { edges: 5, radius: 1 } }); ``` Example of working with turtlesim - explicitly requested on-the-fly message and service definitions now trump definitions from gennodejs. This puts the choice closer to runtime and also is required right now until the md5sum for services is fixed in gennodejs. - Some fixes in action client - Some fixes to on-the-fly message definitions to resemble gennodejs-generated classes - Broke some long lines
chfritz
force-pushed
the
compute_correct_service_md5sums
branch
from
cbbce49 to
7dff255
Compare
|
Did you have a strange rebase possibly? This doesn't look it contains the action parsing changes to me - I'd expect something in |
|
I actually removed all of that once I realized that catkin will generate the action messages already. So the turtle example now uses those .msg files directly. |
|
Ah ok. This looks good to me, tests run fine too. |
Note that this includes PR #23 as that is includes code that I here needed to refactor.