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

Fix light preview parameters #2982

Merged
merged 3 commits into from
Jan 4, 2022
Merged

Conversation

peci1
Copy link
Contributor

@peci1 peci1 commented Apr 22, 2021

Fixes #2981 . See the issue for detailed description.

lights_fixed.mp4

I also wanted to add tests for the inner/outer angle, but they have no getters. Should I add these? Please, also check that the copy of light I make in the test is actually still a pointer to the preview light and is not pointing to the same object light does at the end of the test. I think it should be so, but another pair of eyes would be good :)

Signed-off-by: Martin Pecka <peckama2@fel.cvut.cz>
Copy link
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the fix,

Regarding the inner/outer angles tests, Adding new methods don't break API, I think it's fine to add them, @chapulina or @scpeters Do you agree?

Copy link
Member

@scpeters scpeters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding the inner/outer angles tests, Adding new methods don't break API, I think it's fine to add them, @chapulina or @scpeters Do you agree?

you are welcome to add getters for inner and outer angles if you like

gazebo/rendering/Light.cc Outdated Show resolved Hide resolved
…t GUI creation.

Signed-off-by: Martin Pecka <peckama2@fel.cvut.cz>
@peci1
Copy link
Contributor Author

peci1 commented Jun 29, 2021

I added the getters and added a test for spotlight creation which utilizes the getters.

Signed-off-by: Louise Poubel <louise@openrobotics.org>
@chapulina
Copy link
Contributor

New test passed on CI, merging!

@chapulina chapulina merged commit 94afbb1 into gazebosim:gazebo9 Jan 4, 2022
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.

None yet

4 participants