Compile-time MPSSE command array construction macro

[CirrusNeptune]

For situations where dynamic MPSSE command data layout is not necessary, a declarative macro may be used to safely construct a valid command array at compile-time.

This macro provides implementations of all commands except for set_clock. It parses a rust-like syntax with additional conveniences (i.e. const bindings have fixed-length array types specified automatically).

Additional details are provided in the doc comment for the macro.

[newAM]

Force pushed to rebase onto the latest origin/main before I review.

[CirrusNeptune]

I just realized you can also leverage macro shadowing in a user crate to do command abstractions.

I could include this in the docs and the following test if you think it would be a valuable mention.

#[test]
fn user_abstracted_macro() {
    macro_rules! mpsse {
        // Practical abstraction for SPI devices.
        (@intern $passthru:tt cs_low(); $($tail:tt)*) => {
            mpsse!(@intern $passthru set_gpio_lower(0x0, 0xb); $($tail)*);
        };
        (@intern $passthru:tt cs_high(); $($tail:tt)*) => {
            mpsse!(@intern $passthru set_gpio_lower(0x8, 0xb); $($tail)*);
        };

        // Everything else handled by crate implementation.
        ($($tokens:tt)*) => {
            libftd2xx::mpsse!($($tokens)*)
        };
    }

    mpsse! {
        const (DATA, DATA_READ_LEN) = {
            wait_on_io_high();
            cs_low();
            wait_on_io_low();
            const DATA_RANGE = clock_data(ClockData::MsbPosIn, [11, 22, 33, 44]);
            cs_high();
            send_immediate();
        };
    }
    assert_eq!(DATA.len(), 16);
    assert_eq!(
        DATA,
        [
            MpsseCmd::WaitOnIOHigh as u8,
            MpsseCmd::SetDataBitsLowbyte as u8,
            0x0,
            0xb,
            MpsseCmd::WaitOnIOLow as u8,
            ClockData::MsbPosIn as u8,
            4 as u8,
            0 as u8,
            11 as u8,
            22 as u8,
            33 as u8,
            44 as u8,
            MpsseCmd::SetDataBitsLowbyte as u8,
            0x8,
            0xb,
            MpsseCmd::SendImmediate as u8,
        ]
    );
    assert_eq!(DATA_READ_LEN, 4);
    assert_eq!(DATA_RANGE, 0..4);
}

[newAM]

I just realized you can also leverage macro shadowing in a user crate to do command abstractions.

I could include this in the docs and the following test if you think it would be a valuable mention.

Just saw this after the review; that is really useful and I think it would help some people out!

[CirrusNeptune]

Took a while to read and understand all that macro code, picked up a few new things along the way, pretty elegant!

I highly recommend looking through the examples in https://danielkeep.github.io/tlborm/book/ there are all kinds of useful patterns you should be aware of when designing a declarative macro.

I'll get on implementing those changes in a bit here.

Just saw this after the review; that is really useful and I think it would help some people out!

Should I go ahead and also add the following section to the macro doc?

/// # User Abstractions
///
/// With macro shadowing, it is possible to extend the macro with additional rules for abstract,
/// device-specific commands.
///
/// Comments within the implementation of this macro contain hints on how to implement these rules.
///
/// For example, a SPI device typically delineates transfers with the CS line. Fundamental
/// commands like `cs_high` and `cs_low` can be implmented this way, along with other
/// device-specific abstractions.
///
/// ```no_run
/// # use libftd2xx::mpsse;
/// macro_rules! mpsse {
///     // Practical abstraction of CS line for SPI devices.
///     (@intern $passthru:tt cs_low(); $($tail:tt)*) => {
///         mpsse!(@intern $passthru set_gpio_lower(0x0, 0xb); $($tail)*);
///     };
///     (@intern $passthru:tt cs_high(); $($tail:tt)*) => {
///         mpsse!(@intern $passthru set_gpio_lower(0x8, 0xb); $($tail)*);
///     };
///
///     // Hypothetical device-specific command. Leverages both user and libftd2xx commands.
///     (@intern $passthru:tt
///         const $idx_id:ident = command_42([$($data:expr),* $(,)*]);
///         $($tail:tt)*) => {
///         mpsse!(@intern $passthru
///             cs_low();
///             const $idx_id = clock_data(::libftd2xx::ClockData::MsbPosIn, [0x42, $($data,)*]);
///             cs_high();
///             $($tail)*);
///     };
///
///     // Everything else handled by libftd2xx crate implementation.
///     ($($tokens:tt)*) => {
///         ::libftd2xx::mpsse!($($tokens)*);
///     };
/// }
///
/// mpsse! {
///     const (DATA, DATA_READ_LEN) = {
///         wait_on_io_high();
///         const COMMAND_42_RESULT_RANGE = command_42([11, 22, 33]);
///         send_immediate();
///     };
/// }
/// ```

[newAM]

Should I go ahead and also add the following section to the macro doc?

Yup, looks good, though if it can run I would remove the no_run, just to make sure it does not hit any asserts or such.

[CirrusNeptune]

Hmm, that CI failure is curious.

I get a passing result when running cargo test --all-features --target x86_64-unknown-linux-gnu on my local system with rustc 1.49.0 (e1884a8e3 2020-12-29) and rustc 1.54.0-nightly (5dc8789e3 2021-05-21).

I wonder if there's a macro shadowing bug in the CI compiler version?

EDIT: Yes, when I update to 1.52.1, there's a compiler regression.

[newAM]

Hmm, that CI failure is curious.

I get a passing result when running cargo test --all-features --target x86_64-unknown-linux-gnu on my local system with rustc 1.49.0 (e1884a8e3 2020-12-29) and rustc 1.54.0-nightly (5dc8789e3 2021-05-21).

I wonder if there's a macro shadowing bug in the CI compiler version?

Nice find!

I get it locally on 1.52.1 as well:

error: internal compiler error: failed to process buffered lint here
   --> tests/mpsse-macro.rs:228:45
    |
228 |               ::libftd2xx::mpsse!($($tokens)*);
    |                                               ^
...
232 | /     mpsse! {
233 | |         const (COMMAND_DATA, READ_LEN) = {
234 | |             wait_on_io_high();
235 | |             const COMMAND_42_RESULT_RANGE = command_42([11, 22, 33]);
236 | |             send_immediate();
237 | |         };
238 | |     }
    | |_____- in this macro invocation
    |
    = note: delayed at /rustc/9bc8c42bb2f19e745a63f3445f1ac248fb015e53/compiler/rustc_lint/src/early.rs:393:18
    = note: this error: internal compiler error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

thread 'rustc' panicked at 'no errors encountered even though `delay_span_bug` issued', compiler/rustc_errors/src/lib.rs:1013:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

error: internal compiler error: unexpected panic

note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md

note: rustc 1.52.1 (9bc8c42bb 2021-05-09) running on x86_64-unknown-linux-gnu

note: compiler flags: -C embed-bitcode=no -C debuginfo=2 -C incremental

[CirrusNeptune]

It works in rustc 1.53.0-beta.3 (82b862164 2021-05-22). So it should be fine when the next stable release lands.

[CirrusNeptune]

I have a better macro implementation that's more token hygenic. It does a better job of following the push-down accumulation pattern where the input and output tokens are isolated from each other by the syntax (rather than the shift-append approach). I also got rid of @intern because the syntax is so dang specific, it accomplishes the same purpose of avoiding accidental matches.

(<passthru>) {<input>} -> [<output>]

This guarantees that all input statements are consumed and the macro does not terminate early (without an error at least). It also makes it possible to shadow-override the recursion termination rule. I also put the id and read_len_id passthru tokens into their own token tree so an override can replace them with its own side-channel of tokens.

https://github.com/CirrusNeptune/libftd2xx-rs/compare/mpsse-macro...CirrusNeptune:mpsse-macro-v2

I'm working on a proof of concept for the CC1101 RF module. The write_all and read_all is entirely contained within the macro and custom bitfield types are set in the variable bindings following the read.

I do not think it would be a good idea to provide read/write functionality directly in the libftd2xx macro because there are too many arbitrary details that are best left to the user (expansion scope, error handling, logging/tracing, data types, etc...). Instead maybe a sort of example shadow macro in the docs would be a good idea.

#[test]
fn all_commands_read() {
    let mut ftdi = MockFtdi::new();

    mpsse! {
        ftdi <=> {
            cs_high();
            let sres_status = sres();
            let sidle_status = sidle();
        };
    }
    assert_eq!(sres_status.chip_rdy_n(), false);
    assert_eq!(sres_status.state(), State::Idle);
    assert_eq!(sres_status.fifo_bytes_available(), 0);
    assert_eq!(sidle_status.chip_rdy_n(), false);
    assert_eq!(sidle_status.state(), State::Idle);
    assert_eq!(sidle_status.fifo_bytes_available(), 0);

    assert_eq!(ftdi.write_data, [
        MpsseCmd::SetDataBitsLowbyte as u8,
        0x8 as u8,
        0xb as u8,
        MpsseCmd::SetDataBitsLowbyte as u8,
        0x0 as u8,
        0xb as u8,
        ClockBits::MsbPosIn as u8,
        0x8 as u8,
        Command::SRES as u8,
        MpsseCmd::SetDataBitsLowbyte as u8,
        0x8 as u8,
        0xb as u8,
        MpsseCmd::SetDataBitsLowbyte as u8,
        0x0 as u8,
        0xb as u8,
        ClockBits::MsbPosIn as u8,
        0x8 as u8,
        Command::SIDLE as u8,
        MpsseCmd::SetDataBitsLowbyte as u8,
        0x8 as u8,
        0xb as u8,
        MpsseCmd::SendImmediate as u8,
    ]);
    assert_eq!(ftdi.read_len, 2);
}

Here are the non-generated parts of the macro if you're curious: https://gist.github.com/CirrusNeptune/898881b0520183cc734af9cebb46effe

[CirrusNeptune]

May I go ahead merge the v2 branch into this PR?

[newAM]

May I go ahead merge the v2 branch into this PR?

Yup, looks good to me!

[CirrusNeptune]

I made a really dumb error and didn't subtract 1 from any of the data clocking implementations. Really should have tested with hardware rather than theoretical mock devices.

I'll push a fix in a bit here.

[newAM]

Good catch, I should have tested on live hardware too 🙂

[CirrusNeptune]

Rebased onto main

[newAM]

Looks like 1.53 is coming in two days so we can get this merged then 👍

Someone finally made a website to track this: https://www.whatrustisit.com/ its everything I ever wanted 😄

[newAM]

Rust 1.53 is out! I kicked off CI again, looks like everything is passing. I will test it out with a physical device this weekend and merge it in then.

[CirrusNeptune]

Sounds good!

For reference it looks like this is the related issue: https://github.com/rust-lang/rust/issues/84195