Skip to content

Create MessageMemoryStrategy for subscribers#64

Merged
jacquelinekay merged 1 commit intomasterfrom
msg_memory_strategy
Jul 24, 2015
Merged

Create MessageMemoryStrategy for subscribers#64
jacquelinekay merged 1 commit intomasterfrom
msg_memory_strategy

Conversation

@jacquelinekay
Copy link
Copy Markdown
Contributor

@jacquelinekay jacquelinekay commented Jul 22, 2015

To avoid new whenever a message is received by the subscriber, abstract out the memory strategy for subscribers. MessageMemoryStrategy is templated on the message type and gets passed to the subscriber constructor.

Example usage in ros2/examples#39

@jacquelinekay jacquelinekay added the in progress Actively being worked on (Kanban column) label Jul 22, 2015
@jacquelinekay jacquelinekay mentioned this pull request Jul 22, 2015
Merged
@jacquelinekay jacquelinekay self-assigned this Jul 22, 2015
@jacquelinekay jacquelinekay added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Jul 22, 2015
@jacquelinekay jacquelinekay mentioned this pull request Jul 22, 2015
Closed
7 tasks
wjwwood
wjwwood reviewed Jul 22, 2015
View reviewed changes
rclcpp/include/rclcpp/strategies/single_msg_memory_strategy.hpp Outdated
Copy link
Copy Markdown
Member

@wjwwood wjwwood Jul 22, 2015

Choose a reason for hiding this comment

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

What happens when this is called from multiple threads, i.e. when queue_size is greater than 1 and a multithreaded executor is used?

@jacquelinekay jacquelinekay added in progress Actively being worked on (Kanban column) and removed in review Waiting for review (Kanban column) labels Jul 23, 2015
wjwwood
wjwwood reviewed Jul 23, 2015
View reviewed changes
rclcpp/include/rclcpp/strategies/single_msg_memory_strategy.hpp Outdated
Copy link
Copy Markdown
Member

@wjwwood wjwwood Jul 23, 2015

Choose a reason for hiding this comment

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

I agree, it should be possible to make a solid reset function using the generated code. Though it probably needs to be a free function that takes the message as an argument, to avoid collisions with a message field called reset. Either that or the reset function would need to contain __ somewhere.

@jacquelinekay
Copy link
Copy Markdown
Contributor Author

jacquelinekay commented Jul 23, 2015

summary of changes:

Generalized SingleMsgStrategy to MessagePoolMemoryStrategy.
Implemented placement new for messages to ensure resetting of state.
Keep track of usage state and throw an error if calling code attempts to borrow a message that is currently in use.
Use enable_if to disallow specialization of MessagePoolMemoryStrategy for non-fixed size messages.

@jacquelinekay jacquelinekay added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Jul 23, 2015
dirk-thomas
dirk-thomas reviewed Jul 23, 2015
View reviewed changes
rclcpp/include/rclcpp/strategies/msg_pool_memory_strategy.hpp Outdated
Copy link
Copy Markdown
Member

@dirk-thomas dirk-thomas Jul 23, 2015

Choose a reason for hiding this comment

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

Since all three arrays use the same index wouldn't it make sense to only have one array which contains a struct with three members?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@jacquelinekay jacquelinekay

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jacquelinekay @wjwwood @esteve @dirk-thomas