feat:add example for BLE send message #81
Conversation
|
|
| } | ||
| } | ||
|
|
||
| async fn get_ble_device() -> String { |
There was a problem hiding this comment.
+1 for showing discovery also, when device is not specified in the command line options.
There was a problem hiding this comment.
If Luka wants to have a separate discovery example, that's fine for me. Just having some discovery example would be good.
There was a problem hiding this comment.
I have a discovery example on a local branch, but I didn't like it. I'll try to polish it.
|
Reviewed and commented, in case it helps. Arguably, it could close #63 too? |
I expect this!
Mmmm... @andrewdavidmackenzie is not still not clear for me the design decisions behind the Maybe if I implement some kind of router some day it would have more sense to me? And thanks for the review 🙌 |
|
You've implemented a router here, it's just quite simple, which makes sense as there is not much other app logic that needs it. I think for some simple sending scenarios, not needing a router at all (but supplying from node id) would make sense. |
I would say a "real router", forwarding packets and so on.
Agree |
|
Sorry, I haven't had the chance to have a proper deep look but will try over the weekend. From the first glance, it's 200 lines of code (discounting comments in imports), that's a bit long for an example IMO. Do you think you can make it shorter? Parts of it come from the I've started working on a BLE discovery example but got interrupted by other open source work. I can put it into another file. |
|
@lukipuki I think an example that is self-contained with the minimum needed to do something from beginning to end is very educational — that’s what examples are meant for, in my view — because it shows clearly how things are done. As Andrew said, this is the kind of example I expected to find, so I wrote it. I can strip the discovery part, which is about 2 LOC. Or strip PacketRouter, but what should I do instead? |
|
I think the PacketRouter is minimal and few lines of code. As per my comment: I think that could be removed and FromRadio handled directly, with if let or match or similar. |
|
@andrewdavidmackenzie, I changed a little the example to be more straighforward as also trying to be more precise about ACK handling. Could you check if the ACK handling matches with you knowledge, plz. |
Cargo.toml
Outdated
| version = "0.1.7" | ||
| rust-version = "1.84" | ||
| edition = "2021" | ||
| rust-version = "1.91" |
There was a problem hiding this comment.
That's very recent. Do you need to force people onto such a recent version?
Does something in the code need it?
There was a problem hiding this comment.
Please don't change rust-version in a non-related PR. You can file an issue for that if needed.
There was a problem hiding this comment.
Really only 1.88 is needed to support if let chains. It really makes the code more ergonomic to handle this protobuf de-encapsulations.
https://blog.rust-lang.org/2025/06/26/Rust-1.88.0/
But,why not to bump till last stable.
There was a problem hiding this comment.
Well, you're forcing anyone on 1.88 etc to upgrade before they can run the example, for no real reason?
There was a problem hiding this comment.
it's a draft @andrewdavidmackenzie, I'm just experimenting different options here.
Not forcing anyone to anything.
| stdout().flush().unwrap(); | ||
| }}; | ||
| } | ||
|
|
There was a problem hiding this comment.
I guess you have seen problems due to io not appearing immediately?
There was a problem hiding this comment.
No, it's just that if you print! output is not flushed, and you need it to report progress by printing dots.
| } | ||
| } | ||
|
|
||
| async fn get_ble_device() -> String { |
There was a problem hiding this comment.
If Luka wants to have a separate discovery example, that's fine for me. Just having some discovery example would be good.
|
About BLE discovery I can always add a comment about how to do it! |
4 participants
Summary
Add an example for sending messages over BLE
Related Issues
Closes #78