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.

Sign up for GitHub

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

Correctly initialize result vector in music_rate_in_proxy::get_status() #1458

Merged
merged 4 commits into from
Mar 9, 2020
+2 −1
Merged

Correctly initialize result vector in music_rate_in_proxy::get_status() #1458

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
48d68e5
Corrected variable type
hakonsbm Mar 9, 2020
239335b
Added missing include
hakonsbm Mar 9, 2020
064970c
Using a more minimal change
hakonsbm Mar 9, 2020
ceaaf63
Formatting
hakonsbm Mar 9, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Corrected variable type
@hakonsbm
hakonsbm committed Mar 9, 2020
commit 48d68e5ee0cf373956fcebbc4865cbe19542cc7b
2 changes: 1 addition & 1 deletion models/music_rate_in_proxy.h
View file
Original file line number Diff line number Diff line change
@@ -167,7 +167,7 @@ class music_rate_in_proxy : public DeviceNode

struct Buffers_
{
double data_;
std::vector< double > data_; //!< The buffer for incoming data
Copy link
Contributor

Choose a reason for hiding this comment

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

@hakonsbm Based on your previous analysis this update makes sense, but I wonder if a smaller change, namely changing

( *d )[ names::data ] = DoubleVectorDatum( new std::vector< double >( B_.data_ ) );

to

( *d )[ names::data ] = DoubleVectorDatum( new std::vector< double >( 1, B_.data_ ) );

so that we correctly create a single-element vector, would be the safer approach here to just fix this problem?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. But we should really have a test of this model.

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed, would you create a follow-up issue for a proper review of this (and other music) proxies?

};

// ------------------------------------------------------------