add function for RPC calls

[twendtland]

I made the params string optional, just because I like being able to omit arguments that are not necessary. If someone's not cool with that, I'm happy to make it a normal parameter

[worblehat]

We of course don't know how long the parameters list will be but 64 characters seems very short to me. Maybe increase it a bit to 256 or something like that?

[twendtland]

yeah, you're right, this was the first thing I used for testing, it might well be too short. I will increase it

[worblehat]

The callback will be overwritten by another one if the user calls thingsboad_rpc multiple times without waiting for the response. When the response arrives the wrong callback would be called.
Avoiding multiple RPCs in flight with a semaphore approach that you mentioned yesterday would probably solve this. Or just reset rpc_cb to NULL after it was called and return with an error from thingsboad_rpc() if rpc_cb is not NULL?

A nicer solution would be to allow simultaneous RPC calls by using a memory slab to store multiple callbacks (the same way coap_client.c manages multiple requests).

[twendtland]

yeah, the limitation is obvious. I will think about a solution, being able to do multiple RPC calls simultaneously seems like something that's not often used, but could come in handy.

[worblehat]

I would be fine with having this limitations as it is probably okay for most use cases. We should just document this and somehow enforce the "only one rpc at a time" rule to avoid weird behaviour (like wrong callback called) for the user.

[worblehat]

If you have the time it would be great if you can add a sample for this in samples/, too.

[twendtland]

I added a sample and changed the RPC function to return an error code.

[gumulka]

Not sure, if we should also change our RPC code to already use your code to remove code duplicates and have a seconds example on how to use the RPC callbacks.

[gumulka]

Not sure if the way you inplemented it in the send routine is the right way to do it.

What I assume you want to do is: If there is no response within 5 seconds of sending, then just assume, that there was never a response and send the next message.

What you achieve: When trying to send a message, wait for 5 seconds before actually sending it, so that the old message is gone.

and if two threads try to send at nearly the same time, then the second thread will overwrite the callback for the first thread.

[twendtland]

not sure I can follow. assuming there is no ongoing request: why would the code wait for 5 seconds? 'take' would return immediately, as the semaphore is free. the second param to 'take' is a timeout, after all, isn't it?

[twendtland]

the comparison should be fixed for that, ofc :D

[gumulka]

What I was hinting at was assume the following sequence of Events:
time in minute:seconds

0:00: call to thingsboard_rpc (1)
10:00: call to thingsboard_rpc (2)
10:04: call to thingsboard_rpc (3)

If we assume for 1 to not return, then 2 will wait 5 seconds for the semaphore, and at 10:05 it will reset the semaphore and take it.
Now it will create the request, overwrite the callback and return, making place for other threads to run.
3 will be scheduled to run, because the semaphore it was trying to take was reset, which in turn is an error on taking a semaphore, it will immediately reset the semaphore and take it, then send its own request overwriting the callback.

If we assume the response from 2 at 10:06, it is already to late, as the callback function has been overwritten by 3.

If you instead keep track on when you send out the request at the beginning, then you could reset the semaphore at 10:00, send your request, receive the answer at 10:01 and have no problems or conflicts with 3.