rgot-org / TheThingsNetwork_esp32

82 stars 24 forks source link

join issue #11

Closed rgot-org closed 4 years ago

rgot-org commented 4 years ago

@enwi Hello Moritz i have an issue with join (and provision)

bool TTN_esp32::join( const char* devEui, const char* appEui, const char* appKey, bool force, int8_t retries, uint32_t retryDelay)
{
    restoreKeys();
    force = force
        || (std::equal(dev_eui, dev_eui + 8, devEui) && std::equal(app_eui, app_eui + 8, appEui)
            && std::equal(app_key, app_key + 16, appKey));
    if (force || !provisioned)
    {
        provision(devEui, appEui, appKey);
    }
    return join();
}

I do not understand you introduced "force" and why you compare the parameters with those restored. Normally if we pass the parameters as arguments, it is so that they are processed. On the other hand the comparison does not work because the bytes of dev_eui are reversed compared to devEui idem for appEui and app_eui Can you take a look ? In the meantime I set force to 1 by default.

enwi commented 4 years ago

It was intended so that when you have already provisioned the device that the passed devEui, appEui and appKey are ignored except when the user wants to forcefully update them.

What I think would be better here, to not confuse anybody, is to store the devEui, appEui and appKey to NVM only when one of the provision functions is called and not when any of the join functions is called. The join without any parameters should then only try to retrieve NVM stored devEui, appEui and appKey and the other joins with parameters should set the dev_eui, app_eui and app_key and then join without storing these to NVM

rgot-org commented 4 years ago

My personal idea was not to store the connection informations neither with join nor with provisioning, but that the user has a voluntary process of storage using the saveKeys method and a voluntary process of recovery via restorekeys. In other words the user uses NVS if he want. If you agree, we remove everything about NVS from the join and provision methods.

enwi commented 4 years ago

I would say that the keys should be stored if you use the provision functions, like @manuelbl is doing it in his library https://github.com/manuelbl/ttn-esp32/blob/31b859cdf48d9311af8d3320fc9cdec815ebf080/examples/hello_world/main/main.cpp#L86

That way you can just use the specific join functions if you don't care about storing/provisioning the keys and if you do you use the provision functions and then use the join without any parameters