-
Notifications
You must be signed in to change notification settings - Fork 409
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Unable to Send and receive multiple values with different timestamps of the same resource #1192
Comments
I tried to update ReadResponse to allow send list of timestamped nodes insead of one node but it turned out it requires a lot of changes in related classes. Finally, even if I resolved conflicts (and break a lot of code) I end up on firing LwM2mEncoder.encodeTimestampedData() method with data type |
Send timestamped value is relative to
Yep About implementing
You talk about Send Operation, right ? |
Yes, my main purpose is to allow to Send multiple timestamped values for some path. For example device can collect some of data and send this with historical values. I'm not sure if it's allowed with specification either. |
Do you find any hint in the specification or in https://github.com/OpenMobileAlliance/OMA_LwM2M_for_Developers/issues which could make you think this is allowed ? For now I found nothing. There is maybe a little hint which could me think this is not allowed : We can open at OMA but not sure we get an answer. |
I opened an issue at OpenMobileAlliance/OMA_LwM2M_for_Developers#538 |
We get an answer from OMA. We should first define API directly exposed to users like |
@sbernard31 What do you think, how Send with timestamp implementation should look like? I created branch opl/send_timestamp with POC to show the easiest way to add timestamp to Send command (and incidentally Read). What do you think, it's a good way? Another question is: is it possible to send multiple timestamped values with Send? (I mean one resource with historical values send at once). It looks much more complicated than just add timestamp to single value. I'm not sure if it's even possible within LwM2M specification. |
Following, OpenMobileAlliance/OMA_LwM2M_for_Developers#538 (comment), I would says it's possible. Do you understand the same thing ?
We already have a kind of API TimestampedLwM2mNode used for historical Notification (See ObserveResponse) Concretely, at server side this should be about replace LwM2mNode by TimestampedLwM2mNode. At client side this is clearly less easy.... (Not directly linked but I had this new API idea #981. There is no timestamp too ...)
Maybe an approach could be to have a kind of DataCollector which :
|
I think about this again and SendRequest API use a map of path => node. So don't know how this should looks like for now 🤔 :
(@Michal-Wadowski what is the more urgent subject for you ? this or OSCORE ?) |
Good question :) Both are important quite equally, but I think current issue would be easier to implement than OSCORE. So maybe I'll focus on Send first 🙂 According other questions, I'm still wondering how to solve it in best way... |
Tomorrow, I will try to think about this . (Do no hesitate to share your thought) |
Sorry, I was busy with other tasks until now. Now I have time to work on this task. At the beginning I will focus on the server side because it looks easier to handle. I'm not sure if we can use TimestampedNode. Maybe it should be something like Map<Timestamp, LwM2mNode> (TimestampedLwM2mNodeList ?) Unfortunately I don't know how to test client and server separately in integration tests. Splitting this functionality could ease implementation, but I'm not sure if it is even possible 😐. But I'll try. |
Ok, I think I found solution for that 😄 Now I can develop some parts independently which is easier to do. I'll share POC soon 🙂 |
Maybe, do not go deeper in the code before we agree on the API ?
Eventually we can simulate the SendRequest from the client like with one of this way :
|
I agree, we should discuss about API and architecture. I just send POC as an example (one of many) how we can implement this functionality: (opl/send_timestamp). According to separating client/server test - I'm not able to avoid activities like registration etc, but I can inject custom response even if the client is not able to do it yet. Of course it's kind of hack but it works for me (finally it's only POC). So my proposition is to create TimestampedLwM2mNodeList (the name can be always changed) which is resource-like node that contains map timestamp->value. I'm not sure if it should implement LwM2mResource or LwM2mNode interface so I chose simpler option. Introducing TimestampedLwM2mNodeList type seems don't break current implementation. I think the main changes should be done in LwM2mNodeSenMLDecoder so I prepared first test to cover new approach. It doesn't pass yet due to lack of implementation. I know that #981 could bring some improvements, but I think it needs a lot of rewriting. Maybe it could be done after current functionality? |
I added implementation LwM2mNodeSenMLDecoder to handle TimestampedLwM2mNodeList. Now few tests are broken so I temporarily ignore them. I changed TimestampedLwM2mNodeList as LwM2mResource implementation because it looks like more suitable. |
I think we could merge TimestampedLwM2mNode and TimestampedLwM2mNodeList into single class. It can behave like single node with timestamp and getValue() could return first (or last) element of timestamped list and also allow to get full of history if needed. |
Yep not sure this will be done for the 2.0.0. This is just an idea.
I watched this. My concern with this way is that : I mean it looks this doesn't really fit the LWM2M concept :
E.g : an object instance does not hold a TimestampedLwM2mNodeList or TimestampedLwM2mResourceList The idea of the TimestampedLwM2mNode is that you can timestamp a whole node. e.g. if you observe the object 3 and you get an historical notification with several value of this object at different timestamp, you will get a list of Timestamped LwM2mObject. I'm not sure I'm clear 😅 The big picture Let's step back and try to identify where we need this kind of data. I mean several timestamped node :
I guess that's all ? 🤔
I agree with you let aside the 3. way for now. We can imagine : // by timestamp first. (timestamp -> path -> node)
SortedMap<Long, Map <LwM2mPath, LwM2mNode>> timestampedNodes;
// OR
// by path first. (path -> timestamp -> node)
Map<LwM2mPath,SortedMap<Long, LwM2mNode>> timestampedNodes;
// OR
// by path first. (list sorted by timestamp)
Map<LwM2mPath,List<TimestampedLwM2mNode>> timestampedNodes; For ObserveComposite, I guess this is more timestamp first because this is like you merge several notification in one, and so you want to get each separate notification by timestamp For Send Operation, I don't know which one make more sense. Another solution could be to replace this map of map but a custom dedicated class but is this like thinking about 3. (#981) ? If there are some points which are not clear do not hesitate to ask. |
Oh, I see the case. I was focused primarily on SingleResource timestamp variation, but there are more options indeed. The solution have to be more generic though. I'm thinking about #981 (comment) solution to add more flexibility to ease add timestamps to nodes. |
I also wonder what could be the best choice ? 🤔 Some thought about adding timestamp to node :
For those reasons for now, I think it does not pay off to put timestamp on the node itself. (but maybe I missed something?) About a using a container, I think this is my preferred solution for now. // by timestamp first. (timestamp -> path -> node)
SortedMap<Long, Map <LwM2mPath, LwM2mNode>> timestampedNodes;
// OR
// by path first. (path -> timestamp -> node)
Map<LwM2mPath,SortedMap<Long, LwM2mNode>> timestampedNodes;
// OR
// by path first. (list sorted by timestamp)
Map<LwM2mPath,List<TimestampedLwM2mNode>> timestampedNodes; I think the first one should feet better (for reason exposed above, still in #1192 (comment)) Do you see any drawback with this |
I think the good idea is to not enforce implementation (don't break current api) whenever node has timestamp/history or not. My idea is to add timestamp/history information optionally. I mean if server receive one node or many nodes within the same path but differ timestamps (historical nodes) it could use the same api for both. In second scenario server just gets first (or last) node and ignore the other. But if server use historical-aware api it can access the whole history. I think about optionally extend current node implementations (LwM2mObject, LwM2mObjectInstance, LwM2mSingleResource etc) with timestamp/history feature. I'm currently working on POC to show myself if this approach make sense at all. I'll share details of this idea soon. |
I pushed POC opl/timestamp_api with proposition of API. Please ignore broken tests and hacked LwM2mNodeSenMLDecoder implementation for now. Note API for LwM2mSingleResource: I introduced interface which keep original behaviour and two implementations: LwM2mSingleResourceImpl as original implementation and LwM2mSingleResourceTimestamped decorator which provide original behaviour and allows to access historical data if needed and timestamps if available. Ultimately I want to remove TimestampedLwM2mNodeImpl. |
I looked at this and I still see exactly same problem that I try to described above ☝️ (#1192 (comment)) I don't think a TimestampedNode should be a Node.
|
The reason why I wanted to add timestamp behaviour to current Node (TimestampedNode as a Node) is that the interface SendListener uses method |
We just discover that timestamped value are allowed, so I think we can consider that previous API was not adapted. Instead of adding a new method, I would just change this one : void dataReceived(Registration registration, Map<String, LwM2mNode> data, SendRequest request); by something like one of them : // only historical data
void dataReceived(
Registration registration,
SortedMap<Long, Map <LwM2mPath, LwM2mNode>> historicalData,
SendRequest request);
// historical data + most recent one.
// (I'm not sure about what we put in "mostRecentData", so maybe not a good idea)
void dataReceived(
Registration registration,
Map<String, LwM2mNode> mostRecentData, // or lastData
SortedMap<Long, Map <LwM2mPath, LwM2mNode>> historicalData,
SendRequest request);
// we rely on send request API only to get data
void dataReceived(Registration registration, SendRequest request); (Note that SendRequest API need also to be adapted pretty much in a same way) |
I was thinking we can just do pretty much the same think here. So should we a dedicated structure (e.g : TimestampedLwM2mNodes) which can hold several nodes with timestamped or not timestamped value? The only drawback with this solution is that I feel this overlaps a little bit the LwM2mData idea. |
I'm not sure if we should use timestamps as primary key for nodes. For example if we have something like that:
In listener we'll got three time-separated nodes:
Or is it a good approach though 🤔 ?
I think the dedicated classes are better idea than raw types because we have better control over this data and the representation. Maybe in getTimestampedLwM2mNode() we also should return timestamp->node/nodes map-like structure. |
The idea about timestamp first is to match the Observe-Composite case as explained above :
Maybe I was not clear but I understand that each inner notification should contains all node requested by the Observe-Composite request. (see http://www.openmobilealliance.org/release/LightweightM2M/V1_1_1-20190617-A/HTML-Version/OMA-TS-LightweightM2M_Core-V1_1_1-20190617-A.html#6-4-4-0-644-Observe-Composite-Operation) For Send Operation I don't know is the best way.
OR
I'm not sure what is the best, I guess some time you want value by timestamp and sometime by path. |
Ok, now everything is more clear for me. I'll prepare some solution (POC) with container that allows to access both approaches. And if we'll use container we don't need overloaded dataReceived() of SendListener interface. And maybe we can use that with Composite-Observe. It can be connected also with #981 idea later (now I want to focus mainly on current functionality). |
I created branch opl/send_timestamp3 with POC that use LwM2mData container idea. |
I think My only concern is that Currently I have on eye on OSCORE and I also try to think about a new better transport layer abstraction. Anyway, maybe it's better to just introduce a more simple / specific Timestamped Nodes API. |
No problem, I'll rename the class to be more specific to the timestamp context I used in these changes. I used the LwM2mData name but I didn't mean to do all the functionalities described in #981. I just only noticed that class I created replaces Lists/Maps of nodes. Sorry for the confusion 😄 |
Ok, I will try to help. |
Thanks a lot 😄
Mainly on server side but I think the client side also have to be supported (even for full integration testing). |
I don't know if you see the current NodeEncoder/Decoder interfaces
Without thinking too much of this I would have added a new interface |
Oh my, you are absolutely right! I was focused too much on how to adjust current encoder/decoder. It ended up with replace many current node structures into LwM2mData/TimestampedLwM2mNodes. But adjusting just Send-around functionality and add specific encoder/decoder will simplify the solution so much. Thanks a lot for this clue 😄! It really helps me a lot! |
Ok, I created final (I hope) solution opl/send_timestamp4 that implements multiple timestamped node decoder and send which uses it. The code is not cleaned yet but I'll clean it up if you agree that this solution is ok. |
(I will look at this afternoon) |
I looked at this quickly, I think the general idea is good. 👍 Just maybe 1 impacting thing, currently all data structure (LwM2mNode, TimestampNode, ....) are immutable and I see that TimestampNodes are not. Maybe we should make it immutable too for consistency ? |
I created PR #1217 with solution of current issue |
@Michal-Wadowski with #1217 and #1218 we done with the server side. Do you plan something more ?
For 1. I think we should create a new dedicated issue. Let me know ;) About this issue, I think we can close it now ? |
Yes, our team will be working on this topic. I'm personally be switching to other project but I'll be helping a little bit.
I agree.
Yes, I'll close issue. |
OK ✔️ I created a dedicated issue : #1228
Such a bad news for Leshan project... 😞 Thx again for your work on Leshan 🙏, I hope you will be able to work again with us. |
It seems to be no way to Send multiple data by client from one resource with different timestamps. Also I don't see possibility to receive this kind of data by server (by
SendListener
).However I can see
LwM2mDecoder.decodeTimestampedData()
andLwM2mEncoder.encodeTimestampedData()
which implementations allow to send timestamped data.I created POC code locally that uses already implemented
decodeTimestampedData()
andencodeTimestampedData()
, so technically it's possible to do that, but it needs more investigation.The text was updated successfully, but these errors were encountered: