nRF24 / RF24Network

OSI Layer 3 Networking for nRF24L01(+) and nRF52x on Arduino and Raspberry Pi
https://nrf24.github.io/RF24Network/
GNU General Public License v2.0
355 stars 164 forks source link

[Request] make `level` parameter to multicast() optional #183

Closed 2bndy5 closed 2 years ago

2bndy5 commented 3 years ago

I had the thought to do this in when porting this lib to pure python. If the level parameter is not specified (using the default value 7 or anything greater than 3), then we can let the multicast() function automatically use the private member multicast_level (set by setup_address() or multicastLevel()) as a default value. This should be helpful because the user has no way of getting the current multicast level in the public API (or maybe I missed something).

It's my understanding that the multicastLevel() sets the network level in which the node is listening for multicasted messages. Making the level parameter optional will relieve the user of having to keep passing the new multicast level to the multicast() function.

ps - I'll be conditionally adding the multicastLevel(), multicast(), and multicastRelay to the python wrapper for completeness. Additionally, I'm also adding some other missing attributes like networkFlags & routeTimeout. I've already added the missing begin(uint16_t) method. However, exposing the frame_buffer member will have to wait for nRF24/pyRF24#3.

TMRh20 commented 3 years ago

Sounds good to me

On Jul 28, 2021, at 5:12 PM, Brendan @.***> wrote:

 I had the thought to do this in when porting this lib to pure python. If the level parameter is not specified, then we can let the multicast() function automatically use the private member multicast_level (set by setup_address() or multicastLevel()) as a default value. This should be helpful because the user has no way of getting the current multicast level in the public API (or maybe I missed something).

It's my understanding that the multicastLevel() sets the network level in which the node is listening for multicasted messages. Making the level parameter optional, will relieve the user of having to keep passing the new multicast level to the multicast() function.

ps - I'll be conditionally adding the multicastLevel(), multicast(), and multicastRelay to the python wrapper for completeness. Additionally, I'm also adding some other missing attributes like networkFlags & routeTimeout (I've already added the missing begin(uint16_t) method).

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

2bndy5 commented 3 years ago

I've decided to fill out the missing docs for private members while everything is still fresh in my head...

In the docs about multicastLevel(), it says that for the level parameter:

Levels 1 to 6 are available

Is this a typo? I thought there could only be 4 network levels (0-3) judging from the behavior of RF24Mesh::requestAddress(). The topology page on network levels is kinda vague because it constantly compares to typical IP addressing scheme (which I take for granted/don't know). For my CirPy docs, I have generated some graphs (using graphviz lib) instead of manually using GIMP, but now I'm wondering if I conceived network levels wrong.

2bndy5 commented 3 years ago

This solution is testing well.

any objections to letting multicastLevel() return the current configured multicast level. say like so

>>>network.multicastLevel()
0
>>>network.multicastLevel(1)
1

This would at least allow mesh users to adjust it programmatically (assuming multicastRelay is on).

TMRh20 commented 3 years ago

I think that must be a typo but there should be 5 multicast levels, 0-4. Nodes n111-n555 make up level 4

On Jul 29, 2021, at 4:42 AM, Brendan @.***> wrote:

 This solution is testing well.

any objections to letting multicastLevel() return the current configured multicast level. say like so

network.multicastLevel() 0 network.multicastLevel(1) 1 This would at least allow mesh users to adjust it programmatically.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

2bndy5 commented 3 years ago

Thanks for the clarification. That was the last point of confusion for me. I'll fix the typo and leave the multicastLevel() function as is (I was overthinking it again).