Action Server Support#65
Conversation
IanTheEngineer
left a comment
IanTheEngineer
left a comment
There was a problem hiding this comment.
Looks good, just a few small comments & logging requests.
|
|
||
| let handle = this._getGoalHandle(newGoalId); | ||
|
|
||
| let AddedGoal |
There was a problem hiding this comment.
I don't see this variable being used elsewhere. Should it be removed?
| this._publishResult(result); | ||
| break; | ||
| default: | ||
| // FIXME: log errors for invalid state transitions |
There was a problem hiding this comment.
As a reference, perhaps toss in this link:
http://wiki.ros.org/actionlib/DetailedDescription#Server_Description
to aid the future fixer :)
There was a problem hiding this comment.
In the mean time, just logging any error about an unknown transition would be helpful...
| else if (resp[0] !== 1) { | ||
| const error = new Error('ROS XMLRPC Error'); | ||
| const msg = resp[1]; | ||
| const error = new Error(`ROS XMLRPC Error: ${msg}`); |
There was a problem hiding this comment.
Is this fixing a separate bug?
There was a problem hiding this comment.
This isn't a bug - this is a response from a remote XMLRPC server that indicates some problem in handling the request. It will happen if you try to get a parameter or lookup a service that doesn't exist.
|
|
||
| 'use strict'; | ||
|
|
||
| const NSEC_TO_SEC = 1e-9; |
There was a problem hiding this comment.
+1 for actual constants!
|
Appears this branch needs to be rebased... |
chris-smith
force-pushed
the
actionServer
branch
from
da892f9 to
e425953
Compare
-Adds interface for ActionServers -Adds experimental ActionServer implementation. I assume there are still bugs here. Protocol generally unspecified - had to hunt through actionlib source code. -Renames ActionClient to ActionClientInterface. -Adds time utilities. -Throw useful errors when invalid message type was specified.
chris-smith
force-pushed
the
actionServer
branch
from
e425953 to
e68811c
Compare
| getActionClient(options) { | ||
| options.nh = this.nh; | ||
| return new ActionClient(options); | ||
| return this.nh.actionClientInterface(options); |
There was a problem hiding this comment.
This doesn't actually work. The invoked function doesn't take an object as argument, it requires the server and type to be broken out. I'll create a PR to fix that.
3 participants
-Adds interface for ActionServers
-Adds experimental ActionServer implementation. I
assume there are still bugs here. Protocol generally
unspecified - had to hunt through actionlib source code.
-Renames ActionClient to ActionClientInterface.
-Adds time utilities.
-Throw useful errors when invalid message type was specified.