[Request] ease computation in logicalToPhysicalAddress()

[2bndy5]

According to the current implementation, the private function is_descendant() is called twice in the condition statements for logicalToPhysicalAddress(); first indirectly called by is_direct_child() and second directly called by logicalToPhysicalAddress(). Consider also that the function direct_child_route_to() presumes the given node is a descendant, but it neglects to actual check for this. Observe, the code in question:

RF24Network/RF24Network.cpp

Lines 923 to 943 in d8bcb22

	if (*directTo > TX_ROUTED) {
	pre_conversion_send_node = *to_node;
	*multicast = 1;
	//if(*directTo == USER_TX_MULTICAST || *directTo == USER_TX_TO_PHYSICAL_ADDRESS){
	pre_conversion_send_pipe = 0;
	//}
	}
	// If the node is a direct child,
	else if (is_direct_child(*to_node)) {
	// Send directly
	pre_conversion_send_node = *to_node;
	// To its listening pipe
	pre_conversion_send_pipe = 5;
	}
	// If the node is a child of a child
	// talk on our child's listening pipe,
	// and let the direct child relay it.
	else if (is_descendant(*to_node)) {
	pre_conversion_send_node = direct_child_route_to(*to_node);
	pre_conversion_send_pipe = 5;
	}

RF24Network/RF24Network.cpp

Lines 998 to 1015 in d8bcb22

	bool RF24Network::is_direct_child(uint16_t node)
	{
	bool result = false;
	// A direct child of ours has the same low numbers as us, and only
	// one higher number.
	//
	// e.g. node 0234 is a direct child of 034, and node 01234 is a
	// descendant but not a direct child
	// First, is it even a descendant?
	if (is_descendant(node)) {
	// Does it only have ONE more level than us?
	uint16_t child_node_mask = (~node_mask) << 3;
	result = (node & child_node_mask) == 0;
	}
	return result;
	}

Solution

use if {} else if { if {} else {} } instead.

    if (*directTo > TX_ROUTED) {
        pre_conversion_send_node = *to_node;
        *multicast = 1;
        //if(*directTo == USER_TX_MULTICAST || *directTo == USER_TX_TO_PHYSICAL_ADDRESS){
        pre_conversion_send_pipe = 0;
        //}
    }
    // If the node is a direct child,
    else if (is_descendant(*to_node)) {
        pre_conversion_send_pipe = 5;  // To its listening pipe 
        if (is_direct_child(*to_node)) {
            // Send directly
            pre_conversion_send_node = *to_node;
        }
        // If the node is a child of a child
        // talk on our child's listening pipe,
        // and let the direct child relay it.
        else {
            pre_conversion_send_node = direct_child_route_to(*to_node);
        }
    }

where is_direct_child() no longer needs to call is_descendant():

bool RF24Network::is_direct_child(uint16_t node)
{
    // A direct child of ours has the same low numbers as us, and only
    // one higher number.
    //
    // e.g. node 0234 is a direct child of 034, and node 01234 is a
    // descendant but not a direct child

    // Does it only have ONE more level than us?
    return (node & (uint16_t)((~node_mask) << 3)) == 0;
}

Alternative solution

I could just get rid of the one-line functions is_descendant(), is_direct_child(), and direct_child_route_to() , then hardcode the results into logicalToPhysicalAddress() because this is the only place they are used. However I like the explanatory comments in these one-liners, and I don't want to take away any conciseness (nor add to a newcomer's inevitable confusion - many of the private functions/members are sorely lacking in helpful comments or unused docs). Not to mention that the compilers should automatically optimize these one-liners as inline (I could be wrong about that though).