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

Allow enum to be defined as int parameter #80

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

owenpark8
Copy link
Contributor

Summary

Closes #(Your issue number here)

What features did you add, bugs did you fix, etc?
Allows:

enum class MoteusAuxNumber : int {
    AUX1 = 1,
    AUX2 = 2,
};

MoteusAuxNumber auxNumber;

ParameterWrapper param("limit_switch_aux_number", auxNumber, MoteusAuxNumber::AUX1);

Also style fixes

Did you add documentation to the wiki?

Yes/No (If not explain why not)

How was this code tested?

Tested to work with the enum above. Also made sure it does not compile with enum's without an underlying int type to reduce undefined behavior.

Did you test this in sim?

Yes/No

Did you test this on the rover?

Yes/No

Did you add unit tests?

Yes/No (If not explain why not)

@owenpark8 owenpark8 self-assigned this Feb 25, 2025
@owenpark8 owenpark8 changed the title Allow enum do be defined as int parameter Allow enum to be defined as int parameter Feb 25, 2025
@owenpark8
Copy link
Contributor Author

Lmk if this is stupid. I like the enums in the brushless code tho :D

@owenpark8
Copy link
Contributor Author

owenpark8 commented Feb 25, 2025

Another fix to make the interface more extensible (if we don't want to keep adding constructors for weird edge cases like this) would be to allow pass by pointer. i.e.:

ParameterWrapper(std::string paramDescriptor, int* variable, int defaultValue = 0) : 
        mType{rclcpp::ParameterType::PARAMETER_INTEGER}, mParamDescriptor{std::move(paramDescriptor)},
        mData{variable}, mDefaultValue{defaultValue}{}

Could allow the user to do funky pointer casting if they wanted to

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

Successfully merging this pull request may close these issues.

1 participant