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

renewAddress() returns zero on timeout #211

Closed
2bndy5 opened this issue Jun 18, 2022 · 5 comments
Closed

renewAddress() returns zero on timeout #211

2bndy5 opened this issue Jun 18, 2022 · 5 comments

Comments

@2bndy5
Copy link
Member

2bndy5 commented Jun 18, 2022

I think I know why the examples originally interpreted a zero value returned from renewAddress() as a failure to connect.

RF24Mesh/RF24Mesh.cpp

Lines 280 to 281 in 535c38f

while (!requestAddress(reqCounter)) {
if (millis() - start > timeout) { return 0; }

If the while loop exits, then the network address is returned which should be 04444 if not connected to mesh.

Proposal

Instead of returning the master node's address (I assume it was originally meant to be a boolean false), the timeout condition should just break out of the while loop (returning 04444).

Alternative Considerations

Otherwise the examples would need to be re-written to interpret both 0 and 04444 as a resulting failure from renewAddress(), but that seems to over-complicate the user code a bit. I'm erring with the "break while" idea since the docs state the function returns a network address (not a boolean).

We could go in the other direction and just make the function strictly return a boolean because users can fetch the assigned address from RF24Mesh::mesh_address. But, this approach might break existing code in the wild if expecting the network address instead of a boolean.

@TMRh20
Copy link
Member

TMRh20 commented Jun 18, 2022

Err, now I know why hot-swapping would sometimes work with the old examples. It WAS returning 0. I agree, it should just break out of the loop and return MESH_DEFAULT_ADDRESS if it fails.

@2bndy5
Copy link
Member Author

2bndy5 commented Jun 18, 2022

Mind if I just push a simple commit to master? I don't see this function in RF24Ethernet or RF24Gateway src.

I just noticed the Gateway examples should be updated about renewAddress() return data. The Ethernet examples have already been updated.

@TMRh20
Copy link
Member

TMRh20 commented Jun 18, 2022

That'd be great. I'm not sure what you mean regarding the Gateway examples, they just call renewAddress() without looking at the return data from what I can see. They use failure handling to restart the mesh if required.

@2bndy5
Copy link
Member Author

2bndy5 commented Jun 18, 2022

The ncurses examples use the return value like a bool. They literally do

bool ok = True;

// ...

if ((ok = mesh.rennewAddress())) {
    // ...
}

@TMRh20
Copy link
Member

TMRh20 commented Jun 18, 2022

LOL, I must be blind. I just looked at that thinking it was all cool.

@2bndy5 2bndy5 closed this as completed in a3bc47d Jun 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants