search

ros-naoqi / naoqi_bridge

ROS stack to interact with NAOqi
BSD 3-Clause "New" or "Revised" License
31 stars 45 forks source link

fixing image from the top camera for Pepper #86

Open nlyubova opened 7 years ago

nlyubova commented 7 years ago

The image from top camera is corrupted -> almost white. This PR fix this issue.

@mikaelarguedas and @suryaambrose would you agree to merge it ? or would you prefer to remove the commented code?

mikaelarguedas commented 7 years ago

These parameters are available on nao cameras as far as I can tell so I don't think that this code should be commented out or removed. (Except the one that was already marked as "Might be deprecated"). A better solution would be to have different parameters based on the camera type (or the robot type)

mikaelarguedas commented 7 years ago

Having more information about the issue would help to identify the proper fix. In what condition does this happen ? What is the value of the parameters leading to this "almost white" image ?

nlyubova commented 7 years ago

hello @mikaelarguedas and thank you for your questions.

Here are some answers to your questions: In what condition does this happen ? What is the value of the parameters leading to this "almost white" image ?

-> it happens when starting the package as it is, each time. the default parameters in the code.

when auto_exposition is on or off ?

-> the image is over saturated, we cannot see anything

auto_gain on or off?

-> the image is over saturated, we cannot see anything

if any of them is off: what is the value of the corresponding (exposure/gain) parameter?

-> the image is not good, we cannot see much

what are the other settings of the camera?

-> does not change the image drastically, the image is ok 

how does that compare to the behavior on nao with respect to the camera model used?

-> for Nao and it official last Naoqi -> the default parameters are also too much saturated that almost nothing can be seen.

Since the current code does not work not for Pepper not for Nao I wonder why do we need this parameters? :) by the way it was reported by people doing RoboCup luckily they've seen it

Karsten1987 commented 7 years ago

@nlyubova This is not a real fix. The changes you made so far don't affect the dynamic reconfigure at all, so that people could technically change the values in the GUI, however never encounter any differences in code.

Then further, I hardly believe that these values were never working on NAO, because this code has been around for a while now and was used on NAO. So maybe the default values have changed? What does the Aldebaran documentation say about the default values? That being said, it makes sense to have two different configs, one for NAO, one for pepper.

I also just found this: https://github.com/ros-naoqi/pepper_robot/blob/master/pepper_sensors_py/src/pepper_sensors/pepper_camera.py

Which to my knowledge basically ignores everything in the reconfigure. Could you confirm that? Are you running the nao_sensors_py package for NAO and Pepper?

suryaambrose commented 7 years ago

Hi,

I agree with Karsten here, I am not sure we can say "this does not work for Nao". I remember we had a similar issue at the beginning of the Pepper project, because the cameras of Nao and Pepper are different, with different drivers, and a different set of parameters. I am not surprised that using the same settings for both robots can lead to one not working as expected, and commenting everything out does not look like a fix. IMHO, a more thorough inspection is needed before merging anything

nlyubova commented 7 years ago

Hi @Karsten1987 and @suryaambrose, I agree with you all that it should be done properly to allow a dynamic configure, taking into account a robot type, etc. @Karsten1987 sorry I did not contribute much to pepper_sensors, it seems that it was the initial commit, no?
@suryaambrose right that it works for both robots however makes the image "non-readable white" for Pepper and "over-saturated with lost of details" for Nao.

I only want to let everyone know that the problem exists with these parameters and somebody should fix it. Otherwise, we receive comments from frustrated people that "the official driver does not work" so it is not good.

I'll try to investigate the parameters and let you know.

nlyubova commented 7 years ago

By the way, here is a new web-page for software and doc : https://developer.softbankrobotics.com

mikaelarguedas commented 7 years ago

thanks @Karsten1987 and @suryaambrose for weighing in on this, it looks like we're all on the same page then, with a clear path forward: