search

ros-naoqi / naoqi_dashboard

Visualization (dashboard) for the Nao in ROS
4 stars 7 forks source link

Nao viz #1

Closed vrabaud closed 10 years ago

vrabaud commented 10 years ago

ok, I moved the PR here. If you want to save space, you can:

Now, I have a question before you merge this: Florian suggested that the NaoJointsAnalyzer.cpp be moved to ros_driver. Are you ok with that ? Also, why isn't nao_diagnostic_updater running by default in bringup ?

Ideally, I would like:

ahornung commented 10 years ago

I think nao_diagnostic_updater was not running as default since it did not work with NaoQI 1.12/1.14 until recently. I'm sure that it can be run by default now, best send a PR for that!

What will be the benefit of moving NaoJointsAnalyzer.cpp? I would advocate to run as little as needed directly on the robot and think it's better bundled with the display here. Moving it will introduce pluginlib as dependency in nao_driver, what do you think @severin-lemaignan @danielmaier?

vrabaud commented 10 years ago

Will make a PR for nao_diagnostic_updater (that works for me) NaoJointsAnalyzer.cpp runs at 1Hz and it simply re-parses the output of nao_diagnostic_updater to get more precise info. The reason to move it and run it by default is to have the dashboard be directly loadable in rqt. Right now, you have to roslaunch it (it launches rqt and the NaoJointsAnalyzer.cpp node): that would just bring a convenience. @sosswald probably has an opinion on that too.

vrabaud commented 10 years ago

NaoJointsAnalyzer.cpp does not bring much more info to the table, but it's using so little CPU that it could be worth moving it (the diagnostic part of the dashboard is very usable without that move, just contains a little less info).

ahornung commented 10 years ago

OK :+1:

Well then I guess it's just a matter whether the dependencies diagnostic_aggregator and pluginlib are in robotpkg (or will be) for packaging @severin-lemaignan

severin-lemaignan commented 10 years ago

According to the robotpkg index, both diagnostic_aggregator and pluginlib are already packaged.

vrabaud commented 10 years ago

Hmm, I got news from a parallel work, I'll just ask you to hold on for a few days on that one. The ROS community is amazing :)

ahornung commented 10 years ago

Ah, wrong PR in wrong tab, need to undo... or was this OK now to merge?

vrabaud commented 10 years ago

So actually, now that https://github.com/ros-nao/nao_robot/pull/25 is merged, this one is totally fine as far as I know.

You still have to decide whether you want to squash the first two commits though: that would make a smaller git repo (useless binary data). As nobody was using that repo (the code was broken), I would say go for it.

Can I release it ?

ahornung commented 10 years ago

You still have to decide whether you want to squash the first two commits though: that would make a smaller git repo (useless binary data).

With just a few 100kB that's OK I think ;)

Sure, go ahead and release!

vrabaud commented 10 years ago

Just released in https://github.com/ros/rosdistro/pull/4670