Color [] changed to call by reference

[Avisheet]

Summary

The issue you're describing is related to the operator[] overload for the Color class. The problem is that the operator is declared to return a float instead of a reference to a float. As a result, when you use the operator to access a component of the color and attempt to mutate it, the mutation doesn't apply to the actual color instance.

To fix this issue, you should modify the declaration of the operator[] to return a reference to a float. Here's the corrected declaration:

public: float& operator[](const unsigned int _index);

This modification allows you to use the operator to both access and mutate the individual components of the Color class.

Checklist

• [ ] Signed all commits for DCO
• [ ] Added tests
• [ ] Updated documentation (as needed)
• [ ] Updated migration guide (as needed)
• [ ] Consider updating Python bindings (if the library has them)
• [ ] codecheck passed (See contributing)
• [ ] All tests passed (See test coverage)
• [ ] While waiting for a review on your PR, please help review another open pull request to support the maintainers (optional)

[azeey]

Hello @Avisheet! Thanks for the contribution. The change you made in the header looks correct, but it requires a corresponding change in Color.cc. Would you be able to make that change? Also, could you add a unit test?

[scpeters]

we would probably need to retarget to main as well since this breaks API, unless we add an operator overload instead of changing the current one

[Avisheet]

Hello @azeey . Thank you for your feedback. I've made the necessary changes in Color.cc to align with the modification in Color.hh. Additionally, I have added a corresponding unit test to ensure the functionality is working as expected.

Please review the changes and let me know if everything looks good to you. If you have any further suggestions or if there's anything else you'd like me to address, please feel free to let me know.

[Avisheet]

hello @scpeters , Can you elaborate what you are telling as i am unable to understand it properly

[Avisheet]

@azeey I have done some more changes in the Vector3_TEST.cc file to ensure that we get an expected result as "Operators can be used to [strictly order] vectors."Do check them once and tell me the modifications I need to do if any.

[azeey]

@Avisheet I've added a checklist to your PR description; it should have been prepopulated when you created the PR unless you removed it. Please make sure to complete all relevant items on the checklist.

[Avisheet]

Hey @azeey . The existing behavior is that we return NAN on unknown indices. But if we want to support assignment then it cannot return any such const rvalue . so, we probably have to break that existing functionality . How should we handle that ? As a result of this behavior we are getting failure in 2 test cases.

[Avisheet]

@azeey can you review it and provide me a feedback . So that I can work more in it according to that.

[arjo129]

Hi @Avisheet, was wondering what the status on this was. Do we intend shipping this change with ionic (code freeze 26/8)? If we can't come to a consensus by then I will remove the beta label.

Please address @azeey's feedback on PODs.

[arjo129]

Hi @Avisheet as we haven't heard back from you, I'm removing the beta label. Both @adityapande-1995 and @azeey 's remarks need addressing.

[arjo129]

Hey @Avisheet you don't have to close the PR if you'd like we can merge this in after the code freeze if you can't address it immediately.

[Avisheet]

Hey @arjo129 , sorry for the inconvenience, but as I have some of my major projects going on, I won't be able to contribute to it for 1 more month.

[arjo129]

No worries. I can reopen this we will merge it after your commitments! Its just that we will be putting it in the next gazebo release. Let me know if that works for you or if you don;t want to further pursue this that is also fine.

[Avisheet]

@arjo129 Yep, it's fine to release it with another gazebo release. I will be working on it for sure, but it's just I was a bit stuck with my major project.

[Avisheet]

Hey, I had a doubt that how can I check and run all the test cases on my own device, before giving the PR, so that I can know where the error is, after doing some changes in code.