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

Expose esp_mqtt_client_set_uri. Fix issue #481. #482

Merged
merged 2 commits into from
Sep 7, 2024

Conversation

JakobLachermeier
Copy link
Contributor

Exposes esp_mqtt_client_set_uri in both blocking and async clients.
Fixes #481 by moving unsafe { core::ffi::CStr::from_bytes_with_nul_unchecked(&work.topic) } into their respective commands. I'm not sure why this works.
Doesn't fix #419. I'm also not sure why the original code works on xtensa but not riscv.

Exposes esp_mqtt_client_set_uri in both blocking and async clients.
Fixes esp-rs#481 by moving unsafe { core::ffi::CStr::from_bytes_with_nul_unchecked(&work.topic) }
into  their respective commands. I'm not sure why this works.
@ivmarkov
Copy link
Collaborator

ivmarkov commented Sep 7, 2024

Fixes #481 by moving unsafe { core::ffi::CStr::from_bytes_with_nul_unchecked(&work.topic) } into their respective commands. I'm not sure why this works. Doesn't fix #419. I'm also not sure why the original code works on xtensa but not riscv.

I'm also not sure why this works if it works. It might just be masking out a deeper problem we are not seeing, as moving the topic unwrapping inside the legs of the match is equivalent to the previous code (I mean, if we put aside your new SetUri branch which does not consume the topic and where the value of the topic might be invalid of course).

With that said, I'm OK to merge this. When I have to change the URI, I tend to re-create the whole MQTT client and connection, but I guess having set_uri might be more ergonomic for certain use cases.

@ivmarkov ivmarkov merged commit 32d043e into esp-rs:master Sep 7, 2024
15 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
2 participants