Add tokio codec #18
Add tokio codec #18
Conversation
RaulTrombin
force-pushed
the
Add_tokio_codec
branch
3 times, most recently
from
f2d2d79 to
69a31c8
Compare
|
|
||
| // Assert that the encoded bytes match the original buffer | ||
| assert_eq!(encoded.to_vec(), buffer); | ||
| } |
There was a problem hiding this comment.
Please also add a text to check a wrong buffer fails.
tests/async_codec.rs
Outdated
| eprintln!("Error on serial write: {:?}", e); | ||
| } | ||
| } | ||
| sleep(Duration::from_millis(200)).await; |
There was a problem hiding this comment.
Can we just remove this sleep ?
There was a problem hiding this comment.
tests still fine. Ok
| } | ||
|
|
||
| #[tokio::test] | ||
| #[cfg_attr(not(feature = "local_runner"), ignore)] |
There was a problem hiding this comment.
Can we move this to run_ping_test ?
There was a problem hiding this comment.
different test runtime, also would require pass all dev-dependencies to source, to early, would like to do this after devices.
| ); | ||
|
|
||
| match Messages::try_from(&item) { | ||
| Ok(Messages::Ping1D(ping1d::Messages::DeviceId(answer))) => { |
There was a problem hiding this comment.
We should print all messages.
There was a problem hiding this comment.
ok, fill the match arms for each get method request
tests/async_codec.rs
Outdated
| } | ||
| Ok(_) => {} | ||
| Err(e) => { | ||
| eprintln!("Error on decoder: {:?}", e) |
There was a problem hiding this comment.
panic for both cases: "Unexpected package" and "Error on decoder"
RaulTrombin
force-pushed
the
Add_tokio_codec
branch
from
69a31c8 to
18ec6f9
Compare
tests/async_codec.rs
Outdated
| match write_result { | ||
| Ok(_) => (), | ||
| Err(e) => { | ||
| eprintln!("Error on serial write: {:?}", e); | ||
| } | ||
| } |
There was a problem hiding this comment.
shouldn't we panic here?
tests/async_codec.rs
Outdated
| println!( | ||
| "Received: {} packages, actual package: {:?}", | ||
| { n + 1 }, | ||
| item | ||
| ); |
There was a problem hiding this comment.
can't we use an assert to do this check instead of a println?
There was a problem hiding this comment.
assert!(item.has_valid_crc());
| crate::decoder::DecoderResult::InProgress => { | ||
| consumed += 1; | ||
| if consumed == src.len() { | ||
| src.advance(consumed) | ||
| } | ||
| } | ||
| crate::decoder::DecoderResult::Success(msg) => { | ||
| src.advance(consumed + 1); | ||
| return Ok(Some(msg)); | ||
| } | ||
| crate::decoder::DecoderResult::Error(e) => { | ||
| src.advance(consumed + 1); | ||
| return Err(PingError::ParseError(e)); | ||
| } |
There was a problem hiding this comment.
just checking... The first match variant modifies consumed, but the next two, don't. Is this the intent?
There was a problem hiding this comment.
Yeah, will just advance with (consumed+1(actual_byte)).
and in a specif case, when somehow receive a half package, it will advance with consumed and in next loop will check if a byte is available.
RaulTrombin
force-pushed
the
Add_tokio_codec
branch
from
c7fd841 to
4244fd1
Compare
RaulTrombin
force-pushed
the
Add_tokio_codec
branch
from
4244fd1 to
50ded13
Compare
RaulTrombin
force-pushed
the
Add_tokio_codec
branch
from
50ded13 to
661e9bc
Compare
|
|
||
| fn encode(&mut self, item: ProtocolMessage, dst: &mut BytesMut) -> Result<(), Self::Error> { | ||
| dst.extend_from_slice(&item.serialized()); | ||
| Ok(()) | ||
| } | ||
| } | ||
|
|
||
| #[cfg(test)] |
There was a problem hiding this comment.
This should be squashed with e841a96
| ); | ||
|
|
||
| match Messages::try_from(&item) { | ||
| Ok(Messages::Ping1D(ping1d::Messages::DeviceId(answer))) => { |
RaulTrombin
force-pushed
the
Add_tokio_codec
branch
from
bda3c8d to
4d09484
Compare
RaulTrombin
force-pushed
the
Add_tokio_codec
branch
from
4d09484 to
0cf1121
Compare
This fully implement codec structure using tokio_util Decoder/Encoder.
Add codec and error modules to src.
Codec have it's own tests.
There is an example/test using it with tokio runtime:
cargo test --features local_runnerAlso fails with any panic as the one related in this issue for msg_id=1300.