nRF24 / RF24Mesh

OSI Layer 7 Mesh Networking for RF24Network & nrf24L01+ & nrf52x devices
http://nrf24.github.io/RF24Mesh
GNU General Public License v2.0
422 stars 154 forks source link

why isn't DHCP() automatic? #196

Closed 2bndy5 closed 3 years ago

2bndy5 commented 3 years ago

I've been looking at ways to keep the public API user friendly for my CirPy lib, and I have made the DHCP() call automatic on mesh master node...

I was wondering if there is a reason to not do this for this lib. Does RF24Gateway or RF24Ethernet behave differently when DHCP() is called?

TMRh20 commented 3 years ago

No reason to not do it I think. How did you do it? Adding the dhcp() call to mesh.update?

On Jul 31, 2021, at 7:06 PM, Brendan @.***> wrote:

 I've been looking at ways to keep the public API user friendly for my CirPy lib, and I have made the DHCP() call automatic on mesh master node...

I was wondering if there is a reason to not do this for this lib. Does RF24Gateway or RF24Ethernet behave differently when DHCP() is called?

— 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

all the necessary mechanisms are there, you just have to call DHCP() in RF24Mesh::update() at the end of the "if this is master" block.

    #if !defined(MESH_NOMASTER)
    if (type == NETWORK_REQ_ADDRESS) {
        doDHCP = 1;
    }

    if (!getNodeID()) {
        // handle lookup/release requests
        DHCP();
    }
    #endif //!NO_MASTER

I just checked RF24Ethernet for any references to using DHCP() and it only shows up in an example (with no special behavior performed before or after).

2bndy5 commented 3 years ago

Correction: RF24Gateway calls DHCP() in a function called handleRadioIn()

2bndy5 commented 3 years ago

At first glance it looks like handleRadioIn() calls DHCP() merely out of convention. The rest of the function pulls data from the RF24Network's external data queue.

I don't see how this will cause conflicts (it's definitely backward compatible). I'll make a commit and trigger the Linux workflow for RF24Gateway's CMake-4-Linux branch to spot any compile problems (seriously doubt any problems though).

BTW, all CMake-4-Linux PRs are drafts because they have to be systematically merged in order of OSI layers (waiting on the opened Pico-SDK PR in RF24 repo to get merged). The drafted PRs are all testing changes on the bleeding edge branches of RF24* dependencies, so I have to progressively switch them over to using the master branches in the "merge crusade".

TMRh20 commented 3 years ago

I don’t think there should be any conflicts, but I wonder how much the compile size changes with this?

On Jul 31, 2021, at 8:06 PM, Brendan @.***> wrote:

 At first glance it looks like handleRadioIn() calls DHCP() merely out of convention. The rest of the function pulls data from the RF24Network's external data queue.

I don't see how this will cause conflicts (it's definitely backward compatible). I'll make a commit and trigger the Linux workflow for RF24Gateway's CMake-4-Linux branch to spot any compile problems (seriously doubt any problems though).

BTW, all CMake-4-Linux PRs are drafts because they have to be systematically merged in order of OSI layers (waiting on the opened Pico-SDK PR in RF24 repo to get merged). The drafted PRs are all testing changes on the bleeding edge branches of RF24* dependencies, so I have to progressively switch them over to using the master branches in the "merge crusade".

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

2bndy5 commented 3 years ago

Good point. I haven't been checking that. The following compilation results (for _RF24Mesh_ExampleMaster.ino) all use the Cmake-4-Linux branch of RF24Network and the rp2xxx branch of RF24.

CMake-4-Linux branch of RF24Mesh

  1. ATSAMD21

    Sketch uses 23588 bytes (8%) of program storage space. Maximum is 262144 bytes.

  2. ATMega328

    Sketch uses 10858 bytes (35%) of program storage space. Maximum is 30720 bytes. Global variables use 658 bytes (32%) of dynamic memory, leaving 1390 bytes for local variables. Maximum is 2048 bytes.

with auto-DHCP

  1. ATSAMD21

    Sketch uses 23596 bytes (9%) of program storage space. Maximum is 262144 bytes.

  2. ATMega328

    Sketch uses 10958 bytes (35%) of program storage space. Maximum is 30720 bytes. Global variables use 658 bytes (32%) of dynamic memory, leaving 1390 bytes for local variables. Maximum is 2048 bytes.

on current master branch

  1. ATSAMD21 (with #include <EEPROM.h> commented out)

    Sketch uses 23456 bytes (8%) of program storage space. Maximum is 262144 bytes.

  2. ATMega328

    Sketch uses 10766 bytes (35%) of program storage space. Maximum is 30720 bytes. Global variables use 658 bytes (32%) of dynamic memory, leaving 1390 bytes for local variables. Maximum is 2048 bytes.

EDIT: I was 2 commits behind on RF24Network. Thus the results have been updated to accomodate

2bndy5 commented 3 years ago

I also have an idea about reducing the footprint for these C++ libs (a more object oriented/inheritance approach), but it would be A LOT more work.

2bndy5 commented 3 years ago

if I take the DHCP() call out of the master example, then the size comes down a bit (with auto-DHCP) ATSAMD21

Sketch uses 23588 bytes (8%) of program storage space. Maximum is 262144 bytes.

ATMega328

Sketch uses 10858 bytes (35%) of program storage space. Maximum is 30720 bytes. Global variables use 658 bytes (32%) of dynamic memory, leaving 1390 bytes for local variables. Maximum is 2048 bytes.


Naturally the size goes up for non-master nodes (but that's why MESH_NO_MASTER exists)

without auto-DHCP

ATSAMD21 (CMke-4-Linux branch)

Sketch uses 23124 bytes (8%) of program storage space. Maximum is 262144 bytes. ATMega328 Sketch uses 10280 bytes (33%) of program storage space. Maximum is 30720 bytes. Global variables use 694 bytes (33%) of dynamic memory, leaving 1354 bytes for local variables. Maximum is 2048 bytes.

with auto-DCHP

ATSAMD21

Sketch uses 23588 bytes (8%) of program storage space. Maximum is 262144 bytes.

ATMega328

Sketch uses 11186 bytes (36%) of program storage space. Maximum is 30720 bytes. Global variables use 694 bytes (33%) of dynamic memory, leaving 1354 bytes for local variables. Maximum is 2048 bytes.

TMRh20 commented 3 years ago

Thats not too bad for size. RF24Mesh should compile for ATTiny devices as well or at least I would like to keep it small enough to use them as routing nodes.

On Jul 31, 2021, at 9:22 PM, Brendan @.***> wrote:

 if I take the DHCP() call out of the master example, then the size comes down a bit

Sketch uses 10858 bytes (35%) of program storage space. Maximum is 30720 bytes. Global variables use 658 bytes (32%) of dynamic memory, leaving 1390 bytes for local variables. Maximum is 2048 bytes.

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

2bndy5 commented 3 years ago

I don't have that core installed anymore (did windows "reset" a few months back). I could add the ATTiny to RF24Mesh's Arduino CI workflow, but I would only be concerned about the size for routing nodes when MESH_NO_MASTER is defined. (not sure if we can add that for the CI jobs).

TMRh20 commented 3 years ago

No big deal, just try to be mindful of code size increases.

On Jul 31, 2021, at 9:51 PM, Brendan @.***> wrote:

 I don't have that core installed anymore (did windows "reset" a few months back). I could add the ATTiny to RF24Mesh's Arduino CI workflow, but I would only be concerned about the size for routing nodes when MESH_NO_MASTER is defined. (not sure if we can add that for the CI jobs).

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

2bndy5 commented 3 years ago

Well, I think I've already added enough code size to this lib (solution for my issues with RF24Network), and given the convention has users already calling DHCP() when necessary, I'll leave this idea alone for this lib. Seems like most of my "would-be" improvements are starting to be purely beneficial to my python port.