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

Added GPSExtendedStatus msg to allow for additional status enums #93

Merged
merged 2 commits into from
May 9, 2024

Conversation

agyoungs
Copy link

@agyoungs agyoungs commented Apr 5, 2024

I wanted to add RTK Fix/Float into the GPSStatus message, but #34 already attempted to do this. The drawback is that the md5sum would change and it would potentially be burdensome to all users with previous bag data.

I think a reasonable workaround is the approach I have here. I added a new message that only defines enums. This way anyone can use the extended enums if they wish and they'll be standardized.

@agyoungs agyoungs mentioned this pull request Apr 5, 2024
@danthony06
Copy link
Collaborator

Seems reasonable to me as well.

@danthony06 danthony06 merged commit 68e8398 into swri-robotics:master May 9, 2024
2 of 3 checks passed
@peci1
Copy link

peci1 commented May 9, 2024

Great, thanks for moving on with this! I also suggest to add a reference to the extended fix as a comment to GPSFix. Adding comments doesn't change MD5 sum of the message type.

@agyoungs
Copy link
Author

agyoungs commented May 15, 2024

@peci1 Good point. I went ahead and opened another PR #109 with a comment

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.

3 participants