Skip to content

Conversation

@pulsastrix
Copy link
Member

@pulsastrix pulsastrix commented Jul 16, 2024

This PR rewrites the CoapUri type to work directly with an underlying coap_uri_t object and use the libcoap-provided uri parsing functions.

Additionally the url crate is now an optional dependency, and the CoapUri type now implements FromStr, which allows way easier construction of URIs.

Before (using url crate):

let uri = CoapUri::try_from_url(Url::parse("coap://example.com:4711/foo/bar?answer=42")?)?;

Before (without url crate):

let uri = CoapUri::new(
        CoapUriScheme::Coap,
        Some(CoapUriHost::Name("example.com".to_string())),
        Some(4711),
        Some(vec!["foo".to_string(), "bar".to_string()]),
        Some(vec!["answer=42".to_string()]),,
    );

Now:

let uri: CoapUri = "coap://example.com:4711/foo/bar?answer=42".parse()?;

@pulsastrix pulsastrix requested a review from falko17 July 16, 2024 14:41
@pulsastrix pulsastrix self-assigned this Jul 16, 2024
@github-actions
Copy link

github-actions bot commented Jul 16, 2024

Code Coverage Report

Generated for commit 44d605d on Thu Jul 18 15:34:28 UTC 2024.
Code Coverage

Package Line Rate Health
libcoap/tests/common 88%
libcoap/src/message 37%
libcoap/src/session 53%
libcoap/tests 91%
libcoap-sys 70%
libcoap-sys/src 70%
libcoap/src 44%
Summary 46% (894 / 1946)

@pulsastrix
Copy link
Member Author

Apparently there seems to be some issue with doctests and the unstable --ensure-time cargo flag.

For now, I have disabled these unstable flags and changed the toolchain the tests are run on to stable Rust.

Copy link
Member

@falko17 falko17 left a comment

Choose a reason for hiding this comment

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

Only very few minor comments, but be aware that I'm not that familiar with this repository.

@pulsastrix pulsastrix force-pushed the improve_url_handling branch from d0f274c to 6a0f90e Compare July 18, 2024 10:30
@pulsastrix pulsastrix requested a review from falko17 July 18, 2024 10:31
Copy link
Member

@falko17 falko17 left a comment

Choose a reason for hiding this comment

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

Apart from the two remaining minor comments, LGTM.

@pulsastrix pulsastrix force-pushed the improve_url_handling branch from 6a0f90e to 9def547 Compare July 18, 2024 15:32
@pulsastrix pulsastrix enabled auto-merge July 18, 2024 15:33
@pulsastrix pulsastrix merged commit 489f005 into main Jul 18, 2024
@pulsastrix pulsastrix deleted the improve_url_handling branch December 16, 2024 21:19
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.

3 participants