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.

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

repo: add support for sharing global values between host and device code #13

Merged

Conversation

antoinevg
Copy link
Member

This PR adds a rough proof-of-concept for defining a set of .toml configuration files for values that need to be shared between host & device interfaces.

Currently there are three files:

  • usb.toml -- USB identifiers such as bVendorId, bProductId etc.
  • libgreat.toml -- endpoint addresses and vendor request numbers for the libgreat transport.
  • registers.toml -- TODO @martinling !

While these files could be parsed using arbitrary mechanisms I've added a little bit of sugar for common use:

Python

The cynthion.shared sub-module provides a wrapper around the files which is provisioned at runtime using the built-in tomllib library: shared.py

from cynthion import shared

VENDOR_ID = shared.usb.bVendorId.cynthion

Rust

The rust implementation uses the static-toml crate to provide a hand-rolled wrapper which is provisioned at compile-time: lib.rs

use crate::shared;

let endpoint_address = shared::libgreat::endpoints::bulk_in_address;

Limitations and Considerations

  • I used shared rather than interfaces to avoid confusion with the pre-existing usage of the word for pygreat board definitions. (host/cynthion/board.py / host/cynthion/interfaces/*)
    • That said - there's something to be said for unifying the two mechanisms.
  • Ideally the .toml files would live in a top-level directory such as cynthion.git/shared/ instead of inside the cynthion host module but that'd make the module build process a pain.
  • On the Rust side the toml files get resuscitated as static items, rather than const. Which is somewhat of a pain if you want to define things like Enum values with it.
  • The static-toml crate does horrible things to both numeric types and snake-cased section names. It's not that much effort to maintain a small sanity wrapper though so maybe it doesn't matter too much.

Other approaches

I also futzed around a bit with the uneval crate which had some promise:

https://gist.github.com/antoinevg/28bb9ddae8d6c8580b7e84a708113c8b

The main issues I ran into:

  • We'd need to define and maintain type definitions for each of the toml files. Depending on your perspective, this is either a good or bad thing.
  • It's less ergonomic than static-toml and requires a fair amount of massaging to get it running on std and no_std code.
  • It pulls in serde which, until now, I'm rather proud of us NOT pulling in.
  • All of which I could live with if I could have gotten this approach to work with serde's toml crate which, it looks like, does not provide loose enough lifetime bounds across references (&'static str, because we don't have String on no_std) to make compile time provisioning work. 😩

@antoinevg antoinevg force-pushed the antoinevg/shared-values branch 2 times, most recently from c4f1d15 to 2904fb7 Compare October 2, 2023 09:00
@antoinevg antoinevg changed the title repo: proof-of-concept for sharing values between host and device code repo: add support for sharing global values between host and device code Oct 2, 2023
@antoinevg antoinevg force-pushed the antoinevg/shared-values branch 14 times, most recently from 6ae2e50 to 58e6477 Compare October 4, 2023 08:09
@antoinevg antoinevg marked this pull request as ready for review October 4, 2023 08:09
@antoinevg antoinevg force-pushed the antoinevg/shared-values branch from 58e6477 to 6551112 Compare October 5, 2023 06:29
@antoinevg antoinevg merged commit 1e531c1 into greatscottgadgets:main Oct 5, 2023
@antoinevg antoinevg deleted the antoinevg/shared-values branch October 5, 2023 06:29
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.

1 participant