Include user context in sx127x struct

[morandg]

Hi @dernasherbrezon!

Here is another suggestion. It allows me to get rid of an ugly singleton in my cpp application. Now I can do something like:

void sx127xRxCallback(sx127x* sx127x, uint8_t* data, uint16_t data_length)
{
    auto context = sx127x_get_user_context(sx127x);
    auto* loraDevice = static_cast<LoraDeviceSx127x*>(context);

    assert(loraDevice != nullptr);

    loraDevice->onRx(data, data_length);
}

LoraDeviceSx127x::LoraDeviceSx127x(/* ... */)
{
    auto status = sx127x_create(&mSpiFd, &mSx127x);
    if (status != 0) {
        throw std::runtime_error {"Could not create sx127x device"};
    }
    sx127x_set_user_context(this, &mSx127x);
    sx127x_rx_set_callback(sx127xRxCallback, &mSx127x);
}

void LoraDeviceSx127x::onRx(uint8_t* data, uint16_t data_length)
{
    /* Do stuff with received data */
}

This is just a proposal, let me know if it makes sense to you or do you see a different approach to achieve what I want to do? I can also add some tests when required.

[dernasherbrezon]

Good idea! However normally void *ctx is set together with the callback. Here is very quick example I was able to find:

    BaseType_t xTaskCreatePinnedToCore( TaskFunction_t pxTaskCode,
                                        const char * const pcName,
                                        const uint32_t ulStackDepth,
                                        void * const pvParameters,
                                        UBaseType_t uxPriority,
                                        TaskHandle_t * const pxCreatedTask,
                                        const BaseType_t xCoreID );

In the example above pvParameters will be passed to the callback (task function) pxTaskCode.

On the other hand, if we add something similar to the library, then it will require new major version, because it won't be backward compatible anymore.

Let me think about it.

[morandg]

Thanks for your feedback! I came up with this approach first because it doesn't break the API. Since you are thinking about it, let me share my thoughts.

Probably using the same context for every callback would be sufficient. I wanted first pass it during sx127x_create. Passing it during sx127x_XXX_set_callback would require a different context for each callbacks.

As you mentioned, giving it back during the callback (void (*rx_callback)(void* context, sx127x *, uint8_t *, uint16_t)) seems to be the most common way in many API (pthread_create among many others).

On the other hands, having the sx127x* pointer is duplicated, there should be only sx127x* or void*. With the getter, it avoids too many parameters while keeping the same callback signature.

I'm not sure what would be the best, it's yours to decide 😉 !

[dernasherbrezon]

User context object was included into 5.x release. This release contains several backward incompatible changes including this one.

void sx127x_rx_set_callback(void (*rx_callback)(void *, uint8_t *, uint16_t), void *arg, sx127x *device);

[morandg]

Awesome, thanks for letting me know!