intel / liblwm2m

liblwm2m is an implementation of the LWM2M protocol from the Open Mobile Alliance.
BSD 3-Clause "New" or "Revised" License
44 stars 28 forks source link

Add userData to bufferSend callback. #18

Closed sbernard31 closed 10 years ago

sbernard31 commented 10 years ago

Add a userdata for bufferSendCallback. (the same way monitorCallback as a monitorUserData for LWM2M_SERVER_MODE) (I'm working on a lua binding.)

dnav commented 10 years ago

Hi, In my view, userdata for bufferSendCallback should be part of the void * sessionH. See tests/client/lwm2mclient.c for instance: typedef struct { int sock; struct sockaddr_storage addr; size_t addrLen; } connection_t;

static uint8_t prv_buffer_send(void * sessionH, uint8_t * buffer, size_t length) { size_t nbSent; size_t offset; connection_t * connP = (connection_t*) sessionH; [snip]

You can add any data you need in the connection_t structure. If you have an usecase where this can not work, please expose it.

BTW, please make sure to update the test client and server when modifying the APIs.

Regards

sbernard31 commented 10 years ago

I understand but, I think :

In my use case, at context initialisation I need to keep a reference to a lua function (which will be called in sendbuffercallback). So it's right, I can do the stuff with sessionH, I mean each time I add a server I could store this function reference in the sessionH object, but it looks like more a workarround.

(Sry, I forgot to update client and server. If you think my proposition make sense, I will change the client and server too.)

dnav commented 10 years ago

Allow me to put more thought into this. Actually I'm not entirely satisfied by the current implementation. I can see that userdata are helpful.

sbernard31 commented 10 years ago

No problem :).

I add some input to your reflection ^^. (It's another problem not directly linked to this pull request). Currently when we add a new server (lwm2m_add_server) we give *sessionH. When we close the lwm2m_context_t, we should also release all the sessionH structure. But,

To avoid that, maybe sessionH should be a struct looks like that :

struct _lwm2m_session
{
    void *                userData;
    lwm2m_close_session   closeFunc;
};

or another solution could be to pass a close session callback. (at context initialization or with a setCloseSessionCallback() function)

dnav commented 10 years ago

Hi, I merged your pull request. Actually I can not come up with a more satisfying solution.

sbernard31 commented 10 years ago

I faced another problem with sessionH. I add it here to feed the reflection :)

In my case, the session store in the lwm2m_server_t of the lwm2m_context_t is not the same 'object' than the one passed to lwm2m_handle_packet. So the prv_findServer of observe.c returns alway false. (To be clear, In my case I need to compare sessionH by value, not by reference.)

So maybe, an equals or compare function could be added to _lwm2m_session :

struct _lwm2m_session
{
    void *                userData;
    lwm2m_close_session   closeFunc;
    lwm2m_compare_session compareFunc;
};
dnav commented 10 years ago

Hi,

The session stored in lwm2m_server_t is the one you past to lwm2m_add_server, it must match the one you will pass to lwm2m_handle_packet when you receive a packet from this server. As stated, the session handle must uniquely identify a peer. The core uses the C operator == to discriminate them. When your client receives a packet, it needs to determine if it comes from a new peer or from one it already "knows". If it is a new one, just assign a unique ID to it. If it is an old one, retrieve the ID you assigned to it. Then pass this ID as the sessionH to lwm2m_handle_packet.

In the test client (tests/client/lwm2mclient.c), I first create a connection_t struct identifying the server (line 268) and I pass a pointer to it as the server session handle on line 282. When I receive a packet (line 368), I first determine if it is coming from the server I added before by comparing the IP address and ports (line 392 and tests/utils/connection.c line 71). If it is the same, I call lwm2m_handle_packet. In the test server, as I do not have prior knowledge of clients, I first look in my list of connection_t if one already exists for this IP address and port (tests/server/lwm2mserver.c line 763). If not, I create a new one (line 766). Then lwm2m_handle_packet is called.

In tests/utils/connection.*, I use a pointer to a struct as the session handle just because it is easier. But the sessionH could be an index in an array of connection_t, or the IPv4 address of the peer.

I hope this helps

sbernard31 commented 10 years ago

Thx for this detailed explanation :). I understand your point, and for now I implement my lua binding as you propose. But in my case I still think this is a bit strange to search in the server list the good sessionH then pass it to lwm2m_handle_packet which will search in this same list again.

Maybe have a look to the code could be more explicit :) Here the Lua sample : https://github.com/sbernard31/liblwm2m/blob/luabinding/lua/simplesample.lua As you can see, I don't handle session, just host and port variable.

Here my current code : https://github.com/sbernard31/liblwm2m/blob/55902eb6689c777b18a39d51d8af0ec9d0705769/lua/lua_liblwm2m.c#L216 (I need to find the corresponding sessionH in the server list) Then in liblwm2m, we search if the sessionH is in the server list too : https://github.com/01org/liblwm2m/blob/8d72f04c228053c5a39802473045e7d299d052dc/core/observe.c#L117 Maybe the oriented object approach I propose is not the good solution too. Just a thought :)

dnav commented 10 years ago

OK I understand your point-of-view. You are using the server list to store the session info. This is why you parse this list twice. In the test client and server, I store the session info in my own list. But actually, the parsing task is also done twice. I can see two ways for improving this:

I'll give more thought to this.