search

ros-naoqi / naoqi_driver

c++ bridge based on libqi
Apache License 2.0
52 stars 93 forks source link

add set/get volume (reference nao_audio package) #97

Closed kochigami closed 5 years ago

kochigami commented 6 years ago

added services for getting & setting master volume. This is a new version of https://github.com/ros-naoqi/naoqi_driver/pull/89

reference: http://doc.aldebaran.com/2-5/naoqi/audio/alaudiodevice-api.html#alaudiodevice-api ALAudioDeviceProxy::setOutputVolume and ALAudioDeviceProxy::getOutputVolume

I added the condition of master_volume < 0 or master_volume > 100 so that it is in accordance with nao_audio in nao_interaction. I used srv files in nao_interaction_msgs in nao_interaction.

reference: I sent https://github.com/ros-naoqi/nao_interaction/pull/10

kochigami commented 6 years ago

For the time being, I didn't know how to solve the error below when I did catkin bt. This can't be merged now.

Errors     << naoqi_driver:make /home/kochigami/catkin_ws/logs/naoqi_driver/build.make.136.log
/home/kochigami/catkin_ws/devel/.private/naoqi_driver/lib/libnaoqi_driver.so: undefined reference to `naoqi::service::GetVolumeService::reset(ros::NodeHandle&)'
/home/kochigami/catkin_ws/devel/.private/naoqi_driver/lib/libnaoqi_driver.so: undefined reference to `naoqi::service::SetVolumeService::reset(ros::NodeHandle&)'
/home/kochigami/catkin_ws/devel/.private/naoqi_driver/lib/libnaoqi_driver.so: undefined reference to `naoqi::service::GetVolumeService::GetVolumeService(std::string const&, std::string const&, boost::shared_ptr<qi::Session> const&)'
/home/kochigami/catkin_ws/devel/.private/naoqi_driver/lib/libnaoqi_driver.so: undefined reference to `naoqi::service::SetVolumeService::SetVolumeService(std::string const&, std::string const&, boost::shared_ptr<qi::Session> const&)'
collect2: error: ld returned 1 exit status
make[2]: *** [/home/kochigami/catkin_ws/devel/.private/naoqi_driver/lib/naoqi_driver/naoqi_driver_node] Error 1
make[1]: *** [CMakeFiles/naoqi_driver_node.dir/all] Error 2
make: *** [all] Error 2
kochigami commented 6 years ago

I found the above error was caused by the wrong position of CMakeLists.txt. I just copied modified files from https://github.com/ros-naoqi/naoqi_driver/pull/89 , and then I made a mistake when relocating them. Sorry about that.

I confirmed this pull-request under ubuntu 14.04, ROS indigo, Pepper with NAOqi 2.5.5.5.

roslaunch naoqi_driver naoqi_driver.launch network_interface:=eth1

kochigami@kochigami-ThinkPad-T450:~/catkin_ws/src/naoqi_driver/share$ rosservice call /naoqi_driver/get_master_volume "{}" 
master_volume: 
  data: 30
kochigami@kochigami-ThinkPad-T450:~/catkin_ws/src/naoqi_driver/share$ rosservice call /naoqi_driver/set_master_volume "master_volume:
  data: -100" 

kochigami@kochigami-ThinkPad-T450:~/catkin_ws/src/naoqi_driver/share$ rosservice call /naoqi_driver/get_master_volume "{}" 
master_volume: 
  data: 30
kochigami@kochigami-ThinkPad-T450:~/catkin_ws/src/naoqi_driver/share$ rosservice call /naoqi_driver/set_master_volume "master_volume:
  data: 10" 

kochigami@kochigami-ThinkPad-T450:~/catkin_ws/src/naoqi_driver/share$ rosservice call /naoqi_driver/get_master_volume "{}" 
master_volume: 
  data: 10
kochigami@kochigami-ThinkPad-T450:~/catkin_ws/src/naoqi_driver/share$ rosservice call /naoqi_driver/set_master_volume "master_volume:
  data: 200" 

kochigami@kochigami-ThinkPad-T450:~/catkin_ws/src/naoqi_driver/share$ rosservice call /naoqi_driver/get_master_volume "{}" 
master_volume: 
  data: 10
nlyubova commented 6 years ago

Thank you @kochigami ! Do you have a possibility to check it with ROS Kinetic? and with Nao robot?

kochigami commented 6 years ago

I'm sorry for my late reply.

I've tested with Pepper + ROS kinetic. I haven't tested with NAO + ROS indigo/ kinetic yet. Please wait for a while.

What I did is as follows:

Errors     << naoqi_driver:make /home/pepper/catkin_ws/logs/naoqi_driver/build.make.000.log                                                                                                               
In file included from /home/pepper/catkin_ws/src/naoqi_driver/src/helpers/driver_helpers.cpp:18:0:
/home/pepper/catkin_ws/src/naoqi_driver/src/helpers/driver_helpers.hpp:26:55: fatal error: nao_interaction_msgs/SetAudioMasterVolume.h: No such file or directory
compilation terminated.
make[2]: *** [CMakeFiles/naoqi_driver.dir/src/helpers/driver_helpers.cpp.o] Error 1
make[2]: *** Waiting for unfinished jobs....
In file included from /home/pepper/catkin_ws/src/naoqi_driver/include/../src/converters/converter_base.hpp:25:0,
                 from /home/pepper/catkin_ws/src/naoqi_driver/include/../src/converters/audio.hpp:24,
                 from /home/pepper/catkin_ws/src/naoqi_driver/src/event/audio.hpp:37,
                 from /home/pepper/catkin_ws/src/naoqi_driver/src/event/audio.cpp:30:
/home/pepper/catkin_ws/src/naoqi_driver/include/../src/converters/../helpers/driver_helpers.hpp:26:55: fatal error: nao_interaction_msgs/SetAudioMasterVolume.h: No such file or directory
compilation terminated.
make[2]: *** [CMakeFiles/naoqi_driver.dir/src/event/audio.cpp.o] Error 1
In file included from /home/pepper/catkin_ws/src/naoqi_driver/src/converters/converter_base.hpp:25:0,
                 from /home/pepper/catkin_ws/src/naoqi_driver/src/converters/audio.hpp:24,
                 from /home/pepper/catkin_ws/src/naoqi_driver/src/naoqi_driver.cpp:27:
/home/pepper/catkin_ws/src/naoqi_driver/src/converters/../helpers/driver_helpers.hpp:26:55: fatal error: nao_interaction_msgs/SetAudioMasterVolume.h: No such file or directory
compilation terminated.
make[2]: *** [CMakeFiles/naoqi_driver.dir/src/naoqi_driver.cpp.o] Error 1
make[1]: *** [CMakeFiles/naoqi_driver.dir/all] Error 2
make: *** [all] Error 2
kochigami commented 6 years ago

I also confirmed this pull-request with NAO + ROS indigo, kinetic.

I learned that naoqi_driver package is compiled by using qibuild or catkin. CMakeLists.txt is for qibuild, and CMakeLists_catkin.txt is for catkin. That is why we need to modify CMakeLists.txt and CMakeLists_catkin.txt.

After https://github.com/ros-naoqi/nao_interaction/pull/10 is merged, I think this is ok to be merged.

nlyubova commented 6 years ago

Thank you for testing ! Right, that there are two CMakeLists for qibuild and catkin and sorry for not making it clear before ... I've just merged ros-naoqi/nao_interaction#10

kochigami commented 6 years ago

Thank you for your comment. Next, I will learn how to pass the Travis test because I'm a novice at it :)

Travis error says:

In file included from /catkin_ws/src/naoqi_driver/src/helpers/driver_helpers.cpp:18:0:
/catkin_ws/src/naoqi_driver/src/helpers/driver_helpers.hpp:26:55: fatal error: nao_interaction_msgs/SetAudioMasterVolume.h: No such file or directory
 #include <nao_interaction_msgs/SetAudioMasterVolume.h>
                                                       ^
compilation terminated.
In file included from /catkin_ws/src/naoqi_driver/src/converters/converter_base.hpp:25:0,
                 from /catkin_ws/src/naoqi_driver/src/converters/audio.hpp:24,
                 from /catkin_ws/src/naoqi_driver/src/naoqi_driver.cpp:27:
/catkin_ws/src/naoqi_driver/src/converters/../helpers/driver_helpers.hpp:26:55: fatal error: nao_interaction_msgs/SetAudioMasterVolume.h: No such file or directory
 #include <nao_interaction_msgs/SetAudioMasterVolume.h>
                                                       ^
compilation terminated.
kochigami commented 6 years ago

When I looked into #87, I found I also made a mistake on this pull-request. This somehow works on Pepper, NAO, ros indigo, kinetic though...

I will update this later: my working branch is here: https://github.com/kochigami/naoqi_driver/tree/add-set-get-volume-fix

kochigami commented 6 years ago

I modified function types of get/set Volume like #87. I confirmed with:

nlyubova commented 6 years ago

could you do "git pull --rebase" do get last changes from master?

kochigami commented 6 years ago

Thank you very much for your review. I updated the pull request and confirmed with

In addition, when doing git rebase, I made several commits which are not necessary (modify-functio-type and modify-try-catch). After this pull request is approved, I will squash them. Sorry for my trouble.

nlyubova commented 6 years ago

@kochigami thanks for modifications and no worries about squashing, GitHub can squash when merging :)

kochigami commented 6 years ago

Thank you for your comment. I confirmed this with four conditions above (Pepper/ NAO, indigo/kinetic), but I got an error at Travis. The error says:

In file included from /catkin_ws/src/naoqi_driver/src/helpers/driver_helpers.cpp:18:0:
/catkin_ws/src/naoqi_driver/src/helpers/driver_helpers.hpp:28:41: fatal error: naoqi_bridge_msgs/SetString.h: No such file or directory
 #include <naoqi_bridge_msgs/SetString.h>
                                         ^
compilation terminated.

Maybe releasing naoqi_bridge_msgs is required. I will study it later.

nlyubova commented 6 years ago

Right, I seen this error on Travis and it is related to the last merged pull request. However, we've merged your corresponding PR in naoqi_bridhe_msgs so it should be able to find that file. We also have required naoqi_bridge_msgs dependence in package.xml and CMakeList that is good. I'll check it more precisely, and please let us know if we forgot something

kochigami commented 6 years ago

I learned that new deb file of ros-indigo-naoqi-bridge-msgs is released after about two weeks. In travis, the old version is used, which causes an error. I saw these: http://wiki.ros.org/ShadowRepository http://packages.ros.org/ros/ubuntu/pool/main/r/ros-indigo-naoqi-bridge-msgs/ http://packages.ros.org/ros-shadow-fixed/ubuntu/pool/main/r/ros-indigo-naoqi-bridge-msgs/

After waiting for two weeks, I will try to run travis. (I'm sorry to try open/close this again and again.)

nlyubova commented 6 years ago

Right, it takes some time. I've released it yesterday, no worries about this Travis error

nlyubova commented 6 years ago

It is already stable on buildfarm, so no worries.

kochigami commented 6 years ago

Now deb version of naoqi-bridge-msgs is 0.0.8. I ran Travis again and found the newest version of nao-interaction-msgs is also needed. Sorry about that.

travis log:

In file included from /catkin_ws/src/naoqi_driver/src/helpers/driver_helpers.cpp:18:0:
/catkin_ws/src/naoqi_driver/src/helpers/driver_helpers.hpp:30:55: fatal error: nao_interaction_msgs/SetAudioMasterVolume.h: No such file or directory
 #include <nao_interaction_msgs/SetAudioMasterVolume.h>
                                                       ^
compilation terminated.
kochigami commented 5 years ago

Sorry, I deleted the commits accidentally... I just wanted to delete unnecessary commit to resolve conflicts, but I deleted other commits... I will re-make this PR. I'm very sorry for my mistake.