discover sends a single repeater discovery request and populates the neighbor list; self is excluded#1718
Conversation
| uint8_t data[10]; | ||
| data[0] = CTL_TYPE_NODE_DISCOVER_REQ; // prefix_only=0 | ||
| data[1] = (1 << ADV_TYPE_REPEATER); | ||
| getRNG()->random(&data[2], 4); // tag |
There was a problem hiding this comment.
Should save the 4-byte tag to a member variable, then make sure the response packets match. Otherwise, a bad actor could pollute everyone's neighbors lists with random garbage.
There was a problem hiding this comment.
Possibly with a timeout too. So if the request tag is reused by some other nearby repeater in the future, it isn't using those responses as well. Probably unlikely since there's 4.2 billion combinations for a uint32_t tag, but if the member variable is never cleared, it's still another way to pollute, as anyone can listen to those tags as they go over the air.
|
Please change base branch to 'dev'. |
examples/simple_repeater/MyMesh.cpp
Outdated
| } else { | ||
| strcpy(reply, "Err - ??"); | ||
| } | ||
| } else if (memcmp(command, "discover", 8) == 0) { |
There was a problem hiding this comment.
Could we make it discover.neighbours so it's more explicit that the intention is to discover nearby neighbours? otherwise discover.repeaters so in the future if we wanted to discover other types the discover prefix doesn't gobble up the request on old firmware.
| uint8_t data[10]; | ||
| data[0] = CTL_TYPE_NODE_DISCOVER_REQ; // prefix_only=0 | ||
| data[1] = (1 << ADV_TYPE_REPEATER); | ||
| getRNG()->random(&data[2], 4); // tag |
There was a problem hiding this comment.
Possibly with a timeout too. So if the request tag is reused by some other nearby repeater in the future, it isn't using those responses as well. Probably unlikely since there's 4.2 billion combinations for a uint32_t tag, but if the member variable is never cleared, it's still another way to pollute, as anyone can listen to those tags as they go over the air.
…neighbor list; self is excluded
realtag-github
force-pushed
the
repeater-v1.13-implement-discover
branch
from
ce6d64d to
e8785dd
Compare
|
I like this idea :)
This will be a nice addition! |
… accepts matching repeater responses
Hopefully I did it correctly. it seems to have picked up 2 unrelated commits though. |
|
If you checkout latest dev branch locally, then rebase your discover branch onto the local dev branch, then force push, it should fix it. The PR needs to be opened against out dev branch though. It's currently set to main branch. Regarding the timeout, wonder if we should bump to 60 sec, since there's some chOnky meshes out there now? The repeaters have quite a high delay before sending a reply to the control packets, since lots of nodes can reply to it. |
|
right I forgot to switch it again here. Only showing 2 commits now as it should.
Yeah 30 seconds might be too short. It works in my tests but I am not very far from other repeaters and our mesh is fairly small. |
|
Sweet! One last comment, you have a check for I would say if you're explicitly sending the command, it should still send the request, even if the repeater you're sending the request from has repeat disabled. It should still be able to discover nearby neighbours. I'm currently away for the day, but will be keen to test this on my repeaters through the app later tonight. |
|
Looks good, thanks! Will flash and test on my repeaters tonight :) |
|
Looks good. |
|
Tested and working on my repeaters. The next app update will also have a new |
3 participants
discover sends a single repeater discovery request and populates the neighbor list; self repeater is excluded.
Allows to quickly build a list of neighbors without waiting for repeaters to advert. Useful for right after restart when the list is empty.