Implement Message Type 5 (Volume Coverage Pattern) #15
Conversation
jtfedd
changed the title
danielway
left a comment
danielway
left a comment
There was a problem hiding this comment.
Really great work on this! This message has some interesting encodings and your logic matches the ICD well. The only "blocking" issue is around uom types in the API not being gated by that feature (which CI is reflecting now that I've run it; it will build and generate docs for every permutation of possible features to check that feature gating is correct), and I've included some minor suggestions too.
Please let me know if you don't have time or interest to make those edits: I would be happy to for you.
| CS, // Contiguous Surveillance | ||
| CDW, // Contiguous Doppler with Ambiguity Resolution | ||
| CDWO, // Contiguous Doppler without Ambiguity Resolution | ||
| B, // Batch | ||
| SPP, // Staggered Pulse Pair |
There was a problem hiding this comment.
We could document these strings:
| CS, // Contiguous Surveillance | |
| CDW, // Contiguous Doppler with Ambiguity Resolution | |
| CDWO, // Contiguous Doppler without Ambiguity Resolution | |
| B, // Batch | |
| SPP, // Staggered Pulse Pair | |
| /// Contiguous Surveillance | |
| CS, | |
| /// Contiguous Doppler with Ambiguity Resolution | |
| CDW, | |
| /// Contiguous Doppler without Ambiguity Resolution | |
| CDWO, | |
| /// Batch | |
| B, | |
| /// Staggered Pulse Pair | |
| SPP, |
| #[cfg(feature = "uom")] | ||
| use uom::si::angle::degree; | ||
| #[cfg(feature = "uom")] | ||
| use uom::si::angular_velocity::degree_per_second; | ||
| #[cfg(feature = "uom")] | ||
| use uom::si::f64::{Angle, AngularVelocity}; |
There was a problem hiding this comment.
These can be combined behind a single feature flag:
| #[cfg(feature = "uom")] | |
| use uom::si::angle::degree; | |
| #[cfg(feature = "uom")] | |
| use uom::si::angular_velocity::degree_per_second; | |
| #[cfg(feature = "uom")] | |
| use uom::si::f64::{Angle, AngularVelocity}; | |
| #[cfg(feature = "uom")] | |
| use uom::si::{ | |
| angle::degree, | |
| angular_velocity::degree_per_second, | |
| f64::{Angle, AngularVelocity}, | |
| }; |
|
|
||
| impl ElevationDataBlock { | ||
| /// The elevation angle for this cut | ||
| pub fn elevation_angle(&self) -> Angle { |
There was a problem hiding this comment.
I think any functions returning uom types will need to be gated by a feature flag to omit them from builds without the feature.
There was a problem hiding this comment.
You might notice in some other message types I included a non-uom variant of functions like these with their native representation, in case a consumer either doesn't want to include uom as a dependency or doesn't want to do the conversion work.
There was a problem hiding this comment.
Oops, good catch. I tried to gate the uom stuff but my ide was not very helpful pointing it out...
| #[cfg(not(feature = "uom"))] | ||
| impl Debug for ElevationDataBlock { |
There was a problem hiding this comment.
As these structs have gotten larger and larger, I've started moving towards a pattern where I conditionally add to the debug struct rather than having two variants. It might be less work and reduce the risk of adding a new field to one representation and not the other.
There was a problem hiding this comment.
Oh yeah, that could clean this up a lot.
danielway
left a comment
danielway
left a comment
There was a problem hiding this comment.
Great work, thanks for contributing it!
2 participants
Problem
Message type 5 (Volume Coverage Pattern) contains a lot of useful information for interpreting the individual elevation cuts.
Solution
Decode it!
VCP message contents from `KDMX20240521_215236_V06`
NOTE: Angles are displayed in radians