Test all messages for all dialects: default and random - #378
Conversation
2b950b3 to
ed7b866
Compare
| pub trait MavlinkDialect { | ||
| /// All messages IDs | ||
| const MESSAGE_IDS: &'static [u32]; | ||
| } | ||
|
|
There was a problem hiding this comment.
This seems like isn't useful for users. Maybe define it for tests only?
There was a problem hiding this comment.
Maybe put this behind some other feature? Once I was trying to implement runtime dialect recognition (like what mavp2p does), and this was something I was missing... And although not precisely what was asked, check #291.
There was a problem hiding this comment.
Oh, so it is useful in some cases. Wouldn't it be better to provide this information under impl MavMessage instead? A function like const fn all_ids(&self) -> &'static [u32] would be more accessible to users since they could see it directly on MavMessage without needing to know about/import the *Dialect types. It should be less complexity to maintain as well since we don't have to introduce new trait and types.
There was a problem hiding this comment.
Good idea, I'll try that, thanks!
There was a problem hiding this comment.
Done, but it looks like GitHub Actions is unstable now and failing to test it.
There was a problem hiding this comment.
ok, now it's back working again
ed7b866 to
d8e55a5
Compare
aed3b8e to
c1d691d
Compare
| /// A MAVLink dialect metadata | ||
| /// | ||
| /// Each message sets `MavMessage` enum implements this trait. The [`Dialect`] trait is used to | ||
| /// represent dialects in an abstract way. | ||
| pub trait Dialect | ||
| where | ||
| Self: Sized, | ||
| { | ||
| /// All dialect's messages IDs | ||
| const MESSAGE_IDS: &'static [u32]; | ||
| } |
There was a problem hiding this comment.
I guess I wasn't clear. What I meant was writing a const fn for each MavMessage, like:
impl MavMessage {
const fn all_ids(&self) -> &'static [u32] {
...
}
}without introducing any additional types or variables.
…ll ardupilotmega messages
c1d691d to
a2a8e1e
Compare
2 participants
Depends on #377