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

Fixed setting scan output config #29

Merged
merged 2 commits into from
Aug 6, 2020
Merged

Fixed setting scan output config #29

merged 2 commits into from
Aug 6, 2020

Conversation

hsd-dev
Copy link
Contributor

@hsd-dev hsd-dev commented Aug 4, 2020

Addressing #25

Some params should be set with set_scanoutput_config and not with set_parameter

@PepperlFuchs PepperlFuchs merged commit 028facd into master Aug 6, 2020
@hsd-dev hsd-dev deleted the fix-dyn-params branch August 6, 2020 08:26
@hsd-dev hsd-dev mentioned this pull request Aug 6, 2020
Copy link

@bkurtz bkurtz left a comment

Choose a reason for hiding this comment

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

Seems like generally a step in the right direction, but I don't see anything that will actually restart the data streaming connection to update the start_angle or max_num_points_scan parameters used by the hardware during capture. That seems like it could get kinda complicated, so maybe it's not desirable to support it yet, but it seems like in that case we probably shouldn't support dynamic reconfigure of those parameters at all?

if(product_ != "R2000")
return;
if(state_ != PFState::RUNNING)
return;
protocol_interface_->handle_reconfig(config, level);
Copy link

Choose a reason for hiding this comment

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

Don't we still want to call this?

{
protocol_interface_->handle_reconfig(config, level);
}
pipeline_->set_scanoutput_config(config_);
Copy link

Choose a reason for hiding this comment

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

Just to be clear, this will set the output published to ROS, but I don't see anywhere this causes a change in the data collected from the R2000/R2300, right?

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