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

Add embedded-svc based http transport #654

Merged
merged 1 commit into from
Apr 16, 2024
Merged

Add embedded-svc based http transport #654

merged 1 commit into from
Apr 16, 2024

Conversation

madmo
Copy link
Contributor

@madmo madmo commented Apr 16, 2024

This small pr adds support for sending envelopes using the embedded-svc http transport available on the esp32 microcontroler.

With this in place it is possible to use sentry in rust (with std) based esp32 projects.

Copy link
Member

@Swatinem Swatinem left a comment

Choose a reason for hiding this comment

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

lgtm, left a small suggestion for improvement

sentry/src/transports/embedded_svc_http.rs Outdated Show resolved Hide resolved
@Swatinem
Copy link
Member

well, our CI runs with --all-features, and it looks like that one can’t just simply build this crate on an arbitrary CI system :-(

@madmo
Copy link
Contributor Author

madmo commented Apr 16, 2024

Thanks for the fast response! I will look into adding a sample esp32 project, so that the checks can pass.

@madmo
Copy link
Contributor Author

madmo commented Apr 16, 2024

Had some issues adding a esp32 project to the workspace:

  • Would have polluted the workspace root with platform specific files
  • Would have required building for esp32, but esp32 does not support all other targets

To get it "working" I made the embedded-svc-http feature do nothing if not compiling for a espidf based target. This has the side effect of effectively disabling CI for espidf.

One solution would be to have a separate ci run with only the embedded-svc-http and a espidf target. Does that sound like an acceptable solution?

@Swatinem
Copy link
Member

To get it "working" I made the embedded-svc-http feature do nothing if not compiling for a espidf based target. This has the side effect of effectively disabling CI for espidf.

This is perfect. Given it wouldn’t compile anyway with the feature flag, but without the proper target_os, this should be fine.

One solution would be to have a separate ci run with only the embedded-svc-http and a espidf target. Does that sound like an acceptable solution?

If it can be done with minimal effort, sure. Otherwise, I would rather treat this as a "tier3" platform or however you want to call it. In the sense that it exists, but "we" as the crate maintainers do not guarantee that it won’t break at some point in the future.
We have a similar policy in sentry-native for platforms where we have no realistic way to test it ourselves.

embedded-svc is used on esp32 for doing https
@madmo
Copy link
Contributor Author

madmo commented Apr 16, 2024

Keeping it as tier3 is fine for me. If I find some spare time, I will try and create ci setup, but in an other PR.

@Swatinem Swatinem merged commit 8c701e8 into getsentry:master Apr 16, 2024
12 checks passed
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.

2 participants