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

Failing to start a recipe #124

Closed
davoclavo opened this Issue Jan 3, 2017 · 9 comments

Comments

Projects
None yet
4 participants
@davoclavo

davoclavo commented Jan 3, 2017

@aguadopd reported that recipes can't be started via the UI on the forum

Apparently the UI is sending the following JSON payload when starting a recipe via this bit of code:

{"recipe_id": "test_recipe"}

However, it looks like the API expects the JSON payload to be an array of arguments that will be passed to the service call, such as this:

["test_recipe"]

Here is a curl example request that should work with the current API.

curl -XPOST 'http://10.0.0.8:5984/_openag/api/0.0.1/service/environment_1/start_recipe' \
     -H "Content-Type: application/json" \
     -d '["test_recipe"]'

I am not that familiar with this codebase yet, but I'll try to submit a PR for this soon.

@gordonbrander

This comment has been minimized.

Show comment
Hide comment
@gordonbrander

gordonbrander Jan 10, 2017

Collaborator

@davoclavo let me know if you plan on working on this. It's on my list, so I'll probably make a fix soon if you don't get there first.

Collaborator

gordonbrander commented Jan 10, 2017

@davoclavo let me know if you plan on working on this. It's on my list, so I'll probably make a fix soon if you don't get there first.

@davoclavo

This comment has been minimized.

Show comment
Hide comment
@davoclavo

davoclavo Jan 10, 2017

@gordonbrander I don't want to side track too much, but what are your thoughts on ditching the API software module entirely and rely on rosbridge_suite instead, which runs a server that exposes a websocket interface to interact with all the ROS stuff (Params, Topics, Services). Then on the front end side use roslibjs, which is a thin async JS wrapper to talk to such ws service.

Is there a specific reason to keep the API module as is? The only thing I can think of is figuring out is if it is possible to proxy the websockets via CouchDB.

Regardless, I think you'd be able to fix/test this faster than I would, given that I have no experience with this repo yet.

davoclavo commented Jan 10, 2017

@gordonbrander I don't want to side track too much, but what are your thoughts on ditching the API software module entirely and rely on rosbridge_suite instead, which runs a server that exposes a websocket interface to interact with all the ROS stuff (Params, Topics, Services). Then on the front end side use roslibjs, which is a thin async JS wrapper to talk to such ws service.

Is there a specific reason to keep the API module as is? The only thing I can think of is figuring out is if it is possible to proxy the websockets via CouchDB.

Regardless, I think you'd be able to fix/test this faster than I would, given that I have no experience with this repo yet.

@gordonbrander

This comment has been minimized.

Show comment
Hide comment
@gordonbrander

gordonbrander Jan 10, 2017

Collaborator

@davoclavo we've discussed this, but it's been on the back-burner.

One requirement is that the ROS REST API (whatever powers it) must be able to proxy through CouchDB's server, since we plan to use Couch's authentication system to protect the API as well.

If you want to submit a PR for such a thing, that would be awesome!

cc @LeonChambers

Collaborator

gordonbrander commented Jan 10, 2017

@davoclavo we've discussed this, but it's been on the back-burner.

One requirement is that the ROS REST API (whatever powers it) must be able to proxy through CouchDB's server, since we plan to use Couch's authentication system to protect the API as well.

If you want to submit a PR for such a thing, that would be awesome!

cc @LeonChambers

@LeonChambers

This comment has been minimized.

Show comment
Hide comment
@LeonChambers

LeonChambers Jan 11, 2017

Collaborator

I'm very much in favor of switching to rosbridge as long as CouchDB can proxy it. I imagine this would require a good amount of re-writing on the frontend though. It's pretty easy in the backend.

Collaborator

LeonChambers commented Jan 11, 2017

I'm very much in favor of switching to rosbridge as long as CouchDB can proxy it. I imagine this would require a good amount of re-writing on the frontend though. It's pretty easy in the backend.

@gordonbrander

This comment has been minimized.

Show comment
Hide comment
@gordonbrander

gordonbrander Jan 11, 2017

Collaborator

Assuming rosbridge can do REST/http as well as sockets, patching the UI should be easy. If it's sockets only, it might take a bit more work, but shouldn't be too hard.

The only thing we might want to watch out for is cross-origin restrictions with websockets. Right now the UI is able to talk with the bot through CORS. I'm not sure what the restrictions are on websockets.

Collaborator

gordonbrander commented Jan 11, 2017

Assuming rosbridge can do REST/http as well as sockets, patching the UI should be easy. If it's sockets only, it might take a bit more work, but shouldn't be too hard.

The only thing we might want to watch out for is cross-origin restrictions with websockets. Right now the UI is able to talk with the bot through CORS. I'm not sure what the restrictions are on websockets.

@LeonChambers

This comment has been minimized.

Show comment
Hide comment
@LeonChambers

LeonChambers Jan 11, 2017

Collaborator

"While rosbridge server provides a WebSocket connection, rosbridge itself is not tied to any particular transport layer. You could add a rosbridge TCP package or node, for example, that communicates with rosbridge using TCP sockets. No WebSockets or browsers needed." (from this page). So it looks like normal HTTP isn't supported.

CORS should still work fine since the initial handshake for WebSockets is actually just an HTTP request.

Collaborator

LeonChambers commented Jan 11, 2017

"While rosbridge server provides a WebSocket connection, rosbridge itself is not tied to any particular transport layer. You could add a rosbridge TCP package or node, for example, that communicates with rosbridge using TCP sockets. No WebSockets or browsers needed." (from this page). So it looks like normal HTTP isn't supported.

CORS should still work fine since the initial handshake for WebSockets is actually just an HTTP request.

@davoclavo

This comment has been minimized.

Show comment
Hide comment
@davoclavo

davoclavo Jan 11, 2017

yeah, @LeonChambers is right. rosbridge won't work as long as Couchdb doesn't provide a WS or TCP proxy :(. And I couldn't find a rosbridge HTTP interface either, so perhaps the API software module eventually become that HTTP interface! I bet other projects would use it as well.

Anyways, back to the issue. I got my dev environment setup for openag_ui and fixed that JSON payload by making it an array of values. However, if there is no recipe running when doing so (like on a fresh install) briefly prompts an error message because it always tries to stop the previous recipe and then start the next one. I tried fixing that as well, but I am still not able to get it to work. (Will continue the discussion in the PR.)

davoclavo commented Jan 11, 2017

yeah, @LeonChambers is right. rosbridge won't work as long as Couchdb doesn't provide a WS or TCP proxy :(. And I couldn't find a rosbridge HTTP interface either, so perhaps the API software module eventually become that HTTP interface! I bet other projects would use it as well.

Anyways, back to the issue. I got my dev environment setup for openag_ui and fixed that JSON payload by making it an array of values. However, if there is no recipe running when doing so (like on a fresh install) briefly prompts an error message because it always tries to stop the previous recipe and then start the next one. I tried fixing that as well, but I am still not able to get it to work. (Will continue the discussion in the PR.)

@davoclavo

This comment has been minimized.

Show comment
Hide comment
@davoclavo

davoclavo commented Jan 25, 2017

Fixed by #125 /cc @aguadopd

@davoclavo davoclavo closed this Jan 25, 2017

@aguadopd

This comment has been minimized.

Show comment
Hide comment
@aguadopd

aguadopd Jan 25, 2017

@davoclavo Perfect, thank you!

aguadopd commented Jan 25, 2017

@davoclavo Perfect, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment