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

Rotor position publisher #7

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Conversation

brentyi
Copy link
Contributor

@brentyi brentyi commented Feb 1, 2017

Hi there!

I just added a bit of code for reading and publishing rotor position data from the various sources the VESC makes available. (encoder data is definitely the most useful)

I added a parameter for setting the source of our position data, which is then published to a sensors/rotor_position topic.

Some of the naming is a little confusing, but it was the most readable I could come up with while maintaining consistency with the VESC's firmware.

Feedback would be appreciated!

@mboulet
Copy link
Contributor

mboulet commented Mar 9, 2017

Thanks for submitting the pull request. I missed seeing it. It's pretty extensive - let me take some time to test.

@brentyi
Copy link
Contributor Author

brentyi commented Mar 10, 2017

Sure, thanks!

// "encoder" is probably the most useful
std::string rotor_position_source;
if (private_nh.getParam("rotor_position_source", rotor_position_source)) {
disp_pos_mode mode = DISP_POS_MODE_NONE;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mboulet
one small thing i was wondering about: do you think this mode should be abstracted into vesc_interface, perhaps through a separate function for each display mode?
it feels slightly more "correct" to do so but would introduce some bloat

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.

2 participants