Skip to content

Conversation

@jbangelo
Copy link
Contributor

I scaled back my initial intentions, this is a few small tweaks on top of Gareth's SBP message generator work. I added a sender ID field to each full message struct, and added a very bare bones SBPMessage trait which has a message type constant, and functions for getting and setting the sender ID.

In addition to the message structure I added a simple parser type that reads through a source and tries to extract an SBP message. It's similar to the C parser, except that it buffers data, to try and avoid system call overhead.

Currently there is only parsing capabilities implemented. Eventually it would be nice to add a function to the SBPMessage trait to convert the message into a Vec<u8>, essentially a complete SBP frame.

@jbangelo jbangelo requested a review from silverjam July 17, 2019 23:40
@jbangelo

This comment has been minimized.

@jbangelo
Copy link
Contributor Author

jbangelo commented Jul 22, 2019

Here are some remaining open questions I have:

  1. Do we want to publish this to crates.io immediately? (Could see an argument to work with it a bit internally before publishing)
  2. If we want to publish to crates.io, do we want to do that manually or have travis do it?
  3. Do we see much benefit in generating tests from the YAML spec?

@jbangelo jbangelo force-pushed the jbangelo/rust-dev branch from 35dfd5f to bd6d619 Compare July 22, 2019 16:58
@jbangelo
Copy link
Contributor Author

Here is a todo list:

  • Error handling could be improved. More descriptive error states could be added to make it easier to tell what failed (i.e. CRC failure). Do we want to leverage an error handling library, like Failure?
  • Add SBP output functionality.
  • Move away from byteorder crate for parsing primitive types. The primitive types should all have a from_le_bytes() function as of version 1.32.0

These don't have to be addressed in this PR, but should probably be tracked somewhere. As github issues maybe?

@silverjam
Copy link
Contributor

silverjam commented Jul 22, 2019 via email

@silverjam
Copy link
Contributor

silverjam commented Jul 22, 2019 via email

[dependencies]
byteorder = "1.2.1"
crc16 = "*"
nom = "5.0.0-beta2"
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need a beta version of nom?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not any more. Apparently version 5.0 has been released since I started work on this :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, let's fix this up and merge.

@jbangelo
Copy link
Contributor Author

@silverjam I'll leave figuring out about publishing to crates.io for later then. I'd prefer to get at least one internal project using this to fix out the major issues before pushing it out to the general public.

And for the tests, do you want to see them in this PR? I can work on adding them, but it'll take some time.

Copy link
Contributor

@silverjam silverjam left a comment

Choose a reason for hiding this comment

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

LGTM, we'll add generated tests in a subsequent PR

@jbangelo jbangelo merged commit 057938b into master Jul 23, 2019
@jbangelo jbangelo deleted the jbangelo/rust-dev branch July 23, 2019 15:10
@silverjam
Copy link
Contributor

dancing-ferris

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.

4 participants