Oceanwater 1120 light params switch lights #329
Conversation
…m instead of a custom uniform. This ensures that light intensities go to the correct lights.
|
Test works |
ow_gazebo_plugins/src/OWLightControlPlugin/OWLightControlPlugin.h
Outdated
Show resolved
Hide resolved
ow_gazebo_plugins/src/OWLightControlPlugin/OWLightControlPlugin.cpp
Outdated
Show resolved
Hide resolved
| class OWLightControlPlugin : public gazebo::VisualPlugin | ||
| { | ||
| public: | ||
| OWLightControlPlugin() {} |
There was a problem hiding this comment.
I suspect that the empty constructor (as opposed to =default, or omitting the declaration) will leave the vector members uninitialized, but also that this has no impact here. Still, I think =default is a better choice.
More importantly, I like classes to be explicit about copy and assignment. In this case the defaults are generated, and this could be explicated with = default. If this plugin should not be copied or assigned, = delete takes care of that nicely.
There was a problem hiding this comment.
After looking around I see there is a lot of information and opinions about trivial vs standard-layout vs POD classes. I don't think understanding all of it is within the realm of this PR. If we want to write style rules about when to use = default and = delete I think it should be separate work.
| OWLightControlPlugin() {} | ||
| ~OWLightControlPlugin() {} | ||
|
|
||
| virtual void Load(gazebo::rendering::VisualPtr visual, sdf::ElementPtr sdf); |
There was a problem hiding this comment.
Purely stylistic tip: in declarations where the purpose of the parameter is obvious from the type, the argument names can be omitted, for a shorter line and less to read. E.g you wouldn't do this with a generic type like int or string.
There was a problem hiding this comment.
I have seen this done a lot, but I think code is more clear when the declaration signature reads the same as the definition signature.
Linked Issues:
| Jira Ticket 🎟️ | OCEANWATER-1120 |
Summary of Changes
Test
Without the PR you will see light intensities on top of the lander flicker between lights. With this PR both light intensities stay with the correct lights.