Ping Info Sampling Rate

[CaptKrasno]

Over the past week I made a acoustic_msgs / norbit_msgs to s7k converter.

However, Looking through the s7k spec we are missing some critical information, the sampling_rate.

Aside from s7k compatibility it gives useful information on the "resolution" of your sonar image or detections.

I make a motion to add this to the ping_info message.

-K2

[rolker]

RawSonarImage has a sample_rate, I'm OK with moving it to ping_info to make it avaiable to other messages.

[lauralindzey]

In this context, does "sample rate" == "range resolution", or does it refer to a digitization of the waveform earlier in the pipeline that may be further processed?

[CaptKrasno]

"sample rate" does not directly equal "range resolution" . It refers to smallest delta-t that can be resolved digitally.

If you have the sample_rate you can accurately convert detections or water column between the time domain and the sample number domain.

[lauralindzey]

@CaptKrasno -- just noticed this again as I'm trying to get my inbox empty. I'd be fine with adding this to the SonarInfo message, and I think we need to bump the release anyways to v2.0 (since we changed stuff after 1.0 and a few tools will support the new format) so soon-ish would be a good time.

[CaptKrasno]

I need to revive this thread. I have it attached to a development fork and have been working on it. Here are my thoughts:

1. I still think we should add sample rate to PingInfo for the reasons above
2. I also think we may want to add a field to SonarImageData or RawSonarImage called "bins_per_second" or "sample_rate_compression" where bins_per_second = sample_rate_compression * sample_rate. When sonar's report an image for visualization they don't always report every pixel of the water column. So the "bins per second" reported for a given beam will be less than the sample rate.

For now, I have just changed set sample_rate = "bins_per_second". But I don't think this is a good idea because it indicates that the actually sampling rate of the unit is lower than reality.

@rolker @lauralindzey

[CaptKrasno]

@lauralindzey @rolker any comment on this so I can add it to the upcoming pull request?