namgk / node-red-contrib-ros

Node-RED nodes for connecting to ROS bridge
MIT License
15 stars 15 forks source link

Addition of ros-time and automatically load packages in ros-publish #5

Closed jorritgoos closed 3 years ago

jorritgoos commented 3 years ago

Changes:

jorritgoos commented 3 years ago

@namgk Hi, could you evaluate this request and merge it in the repo which is linked to npm. This would help me in installing this package on other machines. Thanks in advance!

namgk commented 3 years ago

Hi there,

Thanks for the PR.

If the inclusion of rosnodejs is just for using its Time, see if you can include their code directly without having to add a dependency?

Basically just add the Time class and the utils libraries here:

https://github.com/RethinkRobotics-opensource/rosnodejs/blob/kinetic-devel/src/lib/Time.js

and here

https://github.com/RethinkRobotics-opensource/rosnodejs/blob/kinetic-devel/src/utils/time_utils.js

jorritgoos commented 3 years ago

@namgk Hi, thanks for the respose. I included the code from rosnodejs directly.

namgk commented 3 years ago

How do you serve this? I don't see any server code that serving this request.

$.getJSON('packages', function(list) {
jorritgoos commented 3 years ago

I am not quite sure if I understand your question, but next to node-red, I launch a rosbridge_websocket via: roslaunch rosbridge_server rosbridge_websocket.launch.

namgk commented 3 years ago

Can I launch the bridge at a different computer or at a port that is not 80, I don’t see how this can be possible.

On Fri, Nov 20, 2020 at 3:53 AM jorritgoos notifications@github.com wrote:

I am not quite sure if I understand your question, but next to node-red, I launch a rosbridge_websocket via: roslaunch rosbridge_server rosbridge_websocket.launch.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/namgk/node-red-contrib-ros/pull/5#issuecomment-731125645, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHU7ICCSEUJV4BOT2MRLFTSQZKETANCNFSM4TXSLJNQ .

jorritgoos commented 3 years ago

In one terminal I start the node-red node and on a second one I launch the rosbridge_server according to the command below. That command creates websocket server at: ws://0.0.0.0:9090

But also without that server I can select all the compiled msgs in my workspace.

I used snappy-ros as a reference for changing the files.

namgk commented 3 years ago

What I found when I try your code is 404 not found for /packages. This is from the dev console of Chrome and happens when I double click on the node to configure it.

Can you try to just run node red without any ros terminals? I think you might need better error handling in such cases.

On Fri, Nov 20, 2020 at 8:18 AM jorritgoos notifications@github.com wrote:

In one terminal I start the node-red node and on a second one I launch the rosbridge_server according to the command below. That command creates websocket server at: ws://0.0.0.0:9090

But also without that server I can select all the compiled msgs in my workspace.

I used snappy-ros https://github.com/SnappyRobotics/snappy-ros as a reference for changing the files.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/namgk/node-red-contrib-ros/pull/5#issuecomment-731264028, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHU7IDGCI2YVOAFFLGFJZ3SQ2JGVANCNFSM4TXSLJNQ .

jorritgoos commented 3 years ago

When I run only node-red with the command node-red, I can configure the nodes publish, subscribe and time properly without an error. The only error I get is because it cannot connect to the ros-server. The node time also works.

I tested the code on a windows laptop without ROS and then it gives this error: typeerror util.debug is not a function. But this one is also present when I launch node-red with your code.

Do you have tips for me on how to test the code and create better error handling

namgk commented 3 years ago

Hi, I think it's best to give you a maintainer role, so that you can answer any future issue on this package.

Please check the invitation.

Also let me know your npm user so that I can allow you to publish the package too.

jorritgoos commented 3 years ago

I just created an NPM account: Name: goosj Email: jorritgoos@gmail.com

So whats next. Were you able to use my changes without errors or not yet?

namgk commented 3 years ago

I don't have a ros installation to try right now. I can only look at the code and try the node alone.

It's better for you to be the owner of the code so you can test and fix any upcoming bugs.

Cheers

On Wed., Nov. 25, 2020, 8:10 a.m. jorritgoos, notifications@github.com wrote:

I just created an NPM account: Name: goosj Email: jorritgoos@gmail.com

So whats next. Were you able to use my changes without errors or not yet?

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/namgk/node-red-contrib-ros/pull/5#issuecomment-733802239, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHU7IE2ODIC4J2CM7O7ZJTSRUUAPANCNFSM4TXSLJNQ .

jorritgoos commented 3 years ago

Can I merge the pull request then?

And what do I need to do to link this update to NPM such that it can be updated in node-red?

Sorry for all the questions.

namgk commented 3 years ago

no worry, you can do whatever you want now with the repo as you're the maintainer now.

I've given you write access to the npm registry, so you can just do npm login, and then npm publish within the package to publish it.