Skip to content
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

fixing image from the top camera for Pepper #86

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nlyubova
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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 ?

  • when auto_exposition is on or off ?

  • auto_gain on or off?

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

  • what are the other settings of the camera?

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

@nlyubova
Copy link
Member Author

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
Copy link
Contributor

Karsten1987 commented Aug 22, 2017

@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
Copy link
Member

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
Copy link
Member Author

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.

  • For Pepper, the code was tested on Naoqi 2.5.5.5 (from community.ald.softbankrobotics.com and developer.softbankrobotics.com) that seems to be the current and the last publicly available version for Pepper.
  • For Nao, the code was tested on Naoqi 2.1.4.13 (from community.ald.softbankrobotics.com and developer.softbankrobotics.com) that is the last publicly available version for NAO and it this Naoqi did not change since long time.

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

@nlyubova
Copy link
Member Author

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

@mikaelarguedas
Copy link
Member

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:

  • not commenting out the code but look into the current configuration to find a proper fix
  • evaluate current Nao parameters and set better defaults
  • create a different cfg file for Pepper (ideally have one per camera model with sensible default values)
  • adapt the pepper code to update parameters on dynamic reconfigure callbacks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants