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

Feature/base sampling #125

Merged
merged 14 commits into from
Nov 15, 2019
Merged

Feature/base sampling #125

merged 14 commits into from
Nov 15, 2019

Conversation

rikba
Copy link
Collaborator

@rikba rikba commented Nov 11, 2019

This PR introduces (automatic) base station sampling using a least square estimate.

  • The sampling can be initiated using a service call or automatically through rosparam
  • The sampling will average N covariance weighted ECEF samples using a least squares estimator
  • The ECEF position is converted to WGS84 using geotf and set in the piksi firmware
  • Additionally any receiver type can sample its static position using the least squares estimator initiated by a service call

To be reviewed after #124. Adresses base sampling capabilities mentioned in #102.

This code may have a "code smell" as the base station receiver is subscribing to its own ROS topic to indicate when the sampling has finished.

@rikba rikba mentioned this pull request Nov 12, 2019
@michaelpantic
Copy link
Member

Can you merge new master? Then ill have a look ;-)

@rikba
Copy link
Collaborator Author

rikba commented Nov 14, 2019

Done @michaelpantic

michaelpantic
michaelpantic previously approved these changes Nov 14, 2019
Copy link
Member

@michaelpantic michaelpantic left a comment

Choose a reason for hiding this comment

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

LGTM, see nitpicks about comments

@michaelpantic
Copy link
Member

This code may have a "code smell" as the base station receiver is subscribing to its own ROS topic to indicate when the sampling has finished.

Hmm, that's a bit ugly but if its documented nicely somewhere that should be fine. It has the advantage that the sampler node also could be something external if needed.

BTW, how do you decide if sampling should be run at all?

@rikba
Copy link
Collaborator Author

rikba commented Nov 15, 2019

BTW, how do you decide if sampling should be run at all?

First of all every receiver type can sample its static position using the service call sample_position. The base station additionally has a service call to resample the position and write it to the settings overwrite_position (should probably rename). And the base station can automatically sample its position when setting a rosparam.

@rikba
Copy link
Collaborator Author

rikba commented Nov 15, 2019

Thanks for the review. I documented the code behavior and improved the sampling interface.

@rikba rikba merged commit c391a6e into master Nov 15, 2019
@rikba rikba deleted the feature/base_sampling branch November 15, 2019 08:33
@michaelpantic
Copy link
Member

BTW, how do you decide if sampling should be run at all?

First of all every receiver type can sample its static position using the service call sample_position. The base station additionally has a service call to resample the position and write it to the settings overwrite_position (should probably rename). And the base station can automatically sample its position when setting a rosparam.

Awesome, very nice functionality.. thanks for the clarification!

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