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

Jump to bottom

Fix: build: parser: Ping360 transducer return type #55

Merged
merged 2 commits into from
Jul 30, 2024
Merged

Fix: build: parser: Ping360 transducer return type #55

merged 2 commits into from
Jul 30, 2024

Conversation

RaulTrombin
Copy link
Member

Ping360's transducer method is different from previous methods covered on Ping1D device type.

It's a method from MessageDefinitionCategory::Control that returns a DeviceDataStruct.

The following PR covers this case and open a solution for other Set/Control methods that returns a type like Get methods.

Creating your Ping 1D device
Sending transducer command
DeviceDataStruct { mode: 1, gain_setting: 0, angle: 360, transmit_duration: 7, sample_period: 80, transmit_frequency: 700, number_of_samples: 1200, data_length: 1214, data: [176, 4, 67, 139, 186, 216, 237, 252, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 254, 253, 252, 248, 244, 239, 239, 238, 240, 244, 246, 248, 247, 249, 249, 245, 241, 237, 232, 229, 229, 231, 233, 232, 228, 224, 221, 223, 225, 229, 232, 235, 237, 239, 239, 236, 230, 225, 222, 218, 215, 213, 208, 205, 203, 201, 201, 203, 204, 203, 201, 198, 196, 195, 195, 195, 195, 195, 195, 194, 192, 189, 184, 181, 178, 174, 172, 170, 170, 169, 167, 165, 158, 151, 139, 132, 130, 131, 136, 138, 140, 143, 141, 140, 141, 141, 137, 137, 132, 125, 124, 123, 124, 123, 122, 122, 123, 125, 127, 129, 128, 127, 124, 122, 123, 126, 127, 127, 126, 125, 124, 125, 124, 123, 122, 121, 119, 116, 113, 110, 105, 99, 88, 76, 70, 71, 71, 72, 71, 74, 75, 69, 67, 64, 67, 69, 72, 73, 76, 79, 82, 78, 72, 62, 57, 55, 49, 49, 52, 54, 55, 61, 65, 68, 70, 70, 70, 64, 58, 48, 44, 41, 35, 35, 32, 32, 31, 27, 24, 23, 25, 21, 21, 22, 19, 19, 24, 27, 28, 27, 27, 24, 28, 27, 24, 19, 16, 13, 12, 9, 11, 18, 24, 27, 22, 14, 14, 11, 9, 8, 12, 11, 8, 6, 6, 2, 2, 7, 10, 11, 5, 1, 2, 4, 7, 4, 5, 6, 11, 15, 17, 17, 14, 15, 16, 17, 10, 9, 13, 14, 12, 8, 8, 6, 10, 10, 8, 6, 3, 0, 0, 0, 1, 2, 3, 2, 1, 3, 5, 10, 8, 8, 5, 3, 1, 0, 1, 0, 0, 0, 0, 1, 0, 2, 5, 11, 12, 11, 5, 0, 0, 0, 2, 3, 3, 1, 2, 1, 1, 2, 0, 2, 3, 1, 1, 1, 0, 0, 0, 0, 0, 1, 5, 4, 2, 2, 5, 4, 0, 0, 0, 0, 0, 0, 2, 2, 5, 5, 5, 3, 3, 2, 0, 0, 0, 1, 0, 1, 2, 3, 2, 0, 0, 0, 3, 2, 2, 0, 0, 0, 0, 0, 0, 2, 2, 0, 0, 0, 0, 2, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 2, 4, 1, 0, 0, 0, 0, 0, 1, 3, 3, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 3, 2, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 2, 0, 0, 0, 2, 1, 0, 0, 0, 0, 0, 0, 0, 2, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 1, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 2, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0] }

@RaulTrombin RaulTrombin force-pushed the Fix_ping360_transducer_return_type branch from 2610ce0 to 922c9f7 Compare July 29, 2024 14:42
joaoantoniocardoso
joaoantoniocardoso reviewed Jul 29, 2024
View reviewed changes
build/parser.rs Outdated
quote! {
self.get_common().send_message(package).await?;
// Special case where Ping360's config Transducer method returns a DeviceDataStruct
if self.id == 2601 {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe put this number as a const with a descriptive name

Copy link
Member

Choose a reason for hiding this comment

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

In the end we added in the ping-protocol repository an return_message variable that defines if a message is expected from the sensor.

Copy link
Member

Choose a reason for hiding this comment

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

Check: bluerobotics/ping-protocol#209

@RaulTrombin RaulTrombin force-pushed the Fix_ping360_transducer_return_type branch from 922c9f7 to 6b1a7e9 Compare July 29, 2024 21:31
@RaulTrombin
Copy link
Member Author

@patrickelectric @joaoantoniocardoso

PR updated with newer PingProtocol field 'returns_message'.

patrickelectric
patrickelectric requested changes Jul 30, 2024
View reviewed changes
build/parser.rs
if let Some(value) = return_struct_name {
return_type = quote! { #value };
quote! {
let receiver = self.subscribe();
Copy link
Member

Choose a reason for hiding this comment

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

This should be outside the if/else since both scopes use it

Copy link
Member

Choose a reason for hiding this comment

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

This is inside a quote, can't be changed.

patrickelectric
patrickelectric reviewed Jul 30, 2024
View reviewed changes
build/parser.rs Outdated Show resolved Hide resolved
@RaulTrombin RaulTrombin force-pushed the Fix_ping360_transducer_return_type branch 2 times, most recently from 4db7718 to b2569af Compare July 30, 2024 12:57
patrickelectric
patrickelectric reviewed Jul 30, 2024
View reviewed changes
build/parser.rs Outdated
returns_message: value
.get("returns_message")
.and_then(|v| v.as_str())
.filter(|s| !s.is_empty())
Copy link
Member

Choose a reason for hiding this comment

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

How can this be empty ?

Copy link
Member Author

Choose a reason for hiding this comment

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

depends on protocol, removed this checkup

@RaulTrombin RaulTrombin force-pushed the Fix_ping360_transducer_return_type branch from b2569af to cb8fef4 Compare July 30, 2024 13:40
@patrickelectric patrickelectric merged commit c83b6ec into bluerobotics:master Jul 30, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@patrickelectric patrickelectric patrickelectric approved these changes

@joaoantoniocardoso joaoantoniocardoso Awaiting requested review from joaoantoniocardoso

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@RaulTrombin @patrickelectric @joaoantoniocardoso