Skip to content
This repository has been archived by the owner on Sep 3, 2024. It is now read-only.

initial PR to toggle quic support and add esp threading #63

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

suhasHere
Copy link
Collaborator

This PR brings in minimal support to

  • toggle building transport with QUIC as optional
  • add few esp api to get thread stack configuration for hactar messaging

The latter of the 2 will be revisited once we have basic integration working

@suhasHere suhasHere requested a review from TimEvens September 1, 2023 04:26
Copy link
Contributor

@TimEvens TimEvens left a comment

Choose a reason for hiding this comment

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

ESP-IDF doesn't currently build for me because of missing headers.

Curious as to why to use ESP-IDF for just setting thread priority vs native_handle() and pthread_setschedparam()? Is the thought we'll use more of esp-idf?

CMakeLists.txt Outdated
endif()

option(QUIC_TRANSPORT "Enable QUIC Transport (Default ON)" ON)
Copy link
Contributor

Choose a reason for hiding this comment

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

Considering selection of UDP vs QUIC vs H3 vs maybe TCP in the future is at execution time, disabling QUIC or any other transport protocol option would invalidate the execution time config and use-cases to support that.

What was the reason to not compile in the other protocols? Is it that the dependencies with QUIC are too much causing pain for testing? If so, maybe we wrap this into TEST knobs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is a temporary fix to get esp builds going. PicoTLS has been updated to support mbed tls then we don't need this anymore.

src/transport_udp.cpp Outdated Show resolved Hide resolved
CMakeLists.txt Show resolved Hide resolved
@TimEvens TimEvens self-requested a review October 12, 2023 13:12
Copy link
Contributor

@TimEvens TimEvens left a comment

Choose a reason for hiding this comment

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

LGTM, nice to try it out.

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

Successfully merging this pull request may close these issues.

2 participants