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

Move CoAP server setup into module #348

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

chrysn
Copy link
Collaborator

@chrysn chrysn commented Jul 2, 2024

Description

This should eventually move all the required shared setup for CoAP into coapcore.

So far, it only moves everything after the socket setup (see open questions)

Issues/PRs references

Depends on #346.
Contributes-To: #340

Open Questions

Where is the socket setup best moved? My current approach would be to have a new riot-rs-coapcore crate that does the integration (might alsmost be embassy-net-coapcore, but we'll also pull in the RNG in that step).

Is there any better way to do what is effectively an async callback than by introducing a trait in lib.rs? (I've spent about 1h fighting the borrow checker, concluding that I'd probably need a generic associated type like where for<'a> ClientOps: Fn(...) -> ClientFuture<'a>, ClientFuture<'a>: Future, but Rust can't do that).

Change checklist

  • I have cleaned up my commit history and squashed fixup commits.
  • I have followed the Coding Conventions.
  • I have performed a self-review of my own code.
  • I have made corresponding changes to the documentation.

@chrysn chrysn mentioned this pull request Jul 4, 2024
1 task
This merge rejects some changes from c2859ab: The local changes place
all CoAP output into a dedicated message stream. (That stream may
contain defmt information at some point, but that'll need more
infrastructure).
@chrysn chrysn changed the title Move CoAP server setup into coapcore Move CoAP server setup into module Aug 16, 2024
@@ -166,3 +166,7 @@ defmt = [
# usb peripheral support.
stm32-usb = []
stm32-usb-synopsis = []

# Made available so that downsteram crates such as riot-rs-coap don't need to
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# Made available so that downsteram crates such as riot-rs-coap don't need to
# Made available so that downstream crates such as riot-rs-coap don't need to

@@ -141,7 +141,7 @@ jobs:
args: --verbose --locked --features no-boards -p riot-rs -p riot-rs-boards -p riot-rs-chips -p riot-rs-debug -p riot-rs-embassy -p riot-rs-macros -p riot-rs-random -p riot-rs-rt -p riot-rs-threads -p riot-rs-utils

- name: rustdoc
run: RUSTDOCFLAGS='-D warnings' cargo doc -p riot-rs --features no-boards,bench,external-interrupts,threading,random,csprng,hwrng
run: RUSTDOCFLAGS='-D warnings' cargo doc -p riot-rs --features no-boards,bench,external-interrupts,threading,random,csprng,hwrng,coap,net,usb-ethernet
Copy link
Collaborator

@ROMemories ROMemories Aug 20, 2024

Choose a reason for hiding this comment

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

Suggested change
run: RUSTDOCFLAGS='-D warnings' cargo doc -p riot-rs --features no-boards,bench,external-interrupts,threading,random,csprng,hwrng,coap,net,usb-ethernet
run: RUSTDOCFLAGS='-D warnings' cargo doc -p riot-rs --features no-boards,bench,external-interrupts,threading,random,csprng,hwrng,coap,net

I don't think the usb-ethernet feature should be enabled for documentation, as it does not expose any new items; it's only used for internal behavior selection I think. (Regarding the net feature, I'm not sure yet.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants