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

Adding acoustic subsystem examples #236

Merged
merged 5 commits into from
Sep 5, 2022

Conversation

marcoag
Copy link
Member

@marcoag marcoag commented Aug 16, 2022

Adding the acoustic subsystem examples as suggest in #224 (comment).

The related documentation is hosted in the wiki.

Signed-off-by: Marco A. Gutierrez [email protected]

Signed-off-by: Marco A. Gutierrez <[email protected]>
@marcoag marcoag requested a review from hidmic August 16, 2022 06:05
cb
);

while(true) { sleep(1); }
Copy link
Collaborator

Choose a reason for hiding this comment

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

@marcoag IIRC there's a waitForShutdown API in gz::transport.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, added.

Comment on lines 36 to 38
int address = 1;
gz::transport::Node node;
gz::transport::Node::Publisher transmitter;
Copy link
Collaborator

Choose a reason for hiding this comment

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

@marcoag perhaps we move these to the main function below? IIRC gz::transport allows for lambdas as subscription callbacks.

Copy link
Member Author

Choose a reason for hiding this comment

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

Moved the node declaration as is not used in cb. However, I think it's a good idea to keep the example as simple as possible therefore avoid the use of a lambda?

// The data
msg.set_data("test_message");
client.SendPacket(msg);

Copy link
Collaborator

Choose a reason for hiding this comment

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

@marcoag nit: unnecessary blank line.

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed.


int main(int _argc, char **_argv)
{

Copy link
Collaborator

Choose a reason for hiding this comment

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

@marcoag nit: unnecessary blank line.

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed.

@marcoag marcoag requested a review from hidmic August 17, 2022 07:05
Copy link
Collaborator

@hidmic hidmic left a comment

Choose a reason for hiding this comment

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

LGTM but for one last comment

msg.set_type(AcousticMsg::MessageType::LRAUVAcousticMessage_MessageType_Other);
// The data
msg.set_data("test_message");
client.SendPacket(msg);
Copy link
Collaborator

Choose a reason for hiding this comment

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

@marcoag hmm, isn't the service response racing with process shutdown?

Copy link
Member Author

Choose a reason for hiding this comment

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

tbh, I'm not sure... but could be? I mostly copied whatever was there on the documentation example before.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@marcoag could we wait for the response to arrive before exiting?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, just added a sender and a receiver for extra clarity and some more verbosity to the example.

Marco A. Gutierrez added 2 commits August 31, 2022 19:25
@marcoag marcoag mentioned this pull request Sep 2, 2022
22 tasks
@marcoag marcoag merged commit c2546cf into main Sep 5, 2022
@marcoag marcoag deleted the marcoag/acoustic_subsystem_examples branch September 5, 2022 03:22
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