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.

Sign up for GitHub

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

Jump to bottom

Unpack from slice #250

Open
jbbjarnason opened this issue Nov 20, 2024 · 3 comments
Open

Unpack from slice #250

jbbjarnason opened this issue Nov 20, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@jbbjarnason
Copy link
Contributor

I changed type of bar to u32 and get ReadBufferTooShort

thread 'basic_struct' panicked at ethercrab-wire/tests/unpack.rs:28:46:
called `Result::unwrap()` on an `Err` value: ReadBufferTooShort
stack backtrace:
   0: rust_begin_unwind
             at /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/std/src/panicking.rs:652:5
   1: core::panicking::panic_fmt
             at /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/core/src/panicking.rs:72:14
   2: core::result::unwrap_failed
             at /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/core/src/result.rs:1654:5
   3: core::result::Result<T,E>::unwrap
             at /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/core/src/result.rs:1077:23
   4: unpack::basic_struct
             at ./tests/unpack.rs:28:15
   5: unpack::basic_struct::{{closure}}
             at ./tests/unpack.rs:4:18
   6: core::ops::function::FnOnce::call_once
             at /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/core/src/ops/function.rs:250:5
   7: core::ops::function::FnOnce::call_once
             at /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.


failures:
    basic_struct
diff --git a/ethercrab-wire/tests/unpack.rs b/ethercrab-wire/tests/unpack.rs
index 408ad53..0b5490a 100644
--- a/ethercrab-wire/tests/unpack.rs
+++ b/ethercrab-wire/tests/unpack.rs
@@ -8,7 +8,7 @@ fn basic_struct() {
         #[wire(bits = 8)]
         foo: u8,
         #[wire(bits = 16)]
-        bar: u16,
+        bar: u32,
         #[wire(bits = 32)]
         baz: u32,
     }

I am guessing this should work?
@jamwaffles
Copy link
Collaborator

Hm, yes that should still work because #[wire(bits = 16)] is forcing the length. Thanks for the bug report! I'll take a look when I have a mo :)

@jamwaffles jamwaffles added the bug Something isn't working label Nov 20, 2024
@jamwaffles
Copy link
Collaborator

Thinking about this a bit more, I'm hesitant to call this a bug. Perhaps it would be useful to have another derive attribute instead. Something like e.g.

struct Example {
    /// Decode the lower 2 bytes of the field, padding the rest with zeros
    #[wire(bits = 16, pad = 16)]
    field: u32

    /// Equivalent in bytes
    #[wire(bytes = 2, pad_bytes = 2)]
    equivalent: u32
}

I'm still wondering why you need to decode a u16's worth of bytes into a u32 though. Are there two bytes after the field you'd like to discard or something? Would #[wire(post_skip_bytes = N)] help here?

@jbbjarnason
Copy link
Contributor Author

jbbjarnason commented Nov 29, 2024 •
edited
Loading

I just stumbled upon this when I was trying to unpack a fixed size array of u8, but simplified the example here.
Afterwards I could not even use array of u8, since my data from the device was packed on came on bit level and I didn't know how to declare how to transpose bit level to array of u8 or bool.

So I made a workaround using bitview in process loop.

I agree this is a feature.
Can be converted to discussion.

pad and pad_bytes sounds nice, that is if it can work inside an array

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@jamwaffles @jbbjarnason