-
Notifications
You must be signed in to change notification settings - Fork 408
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
SenMLEncoder can not encode Node for multi instance resource with too long resource or resource instance id #1024
Comments
🙏 Thx for reporting this. I will look at this. Which version are you using ?
What do you mean exactly ? 🤔 |
About 2.
I just look at the specification transport-v1.1§6.4.4 - Device Management and Service EnablementInterface and it seems this is not an issue because iPatch MUST be used :
What makes you think PATCH must be used instead ? (I will look at 1. now) |
About 1. I tried to reproduce it, writting a integration test in WriteMultiValueTest : @Test
public void can_write_object_resource_instance() throws InterruptedException {
Map<Integer, String> values = new HashMap<>();
values.put(10, "value10");
values.put(20, "value20");
// write multi instance
WriteResponse response = helper.server.send(helper.getCurrentRegistration(),
new WriteRequest(contentFormat, IntegrationTestHelper.TEST_OBJECT_ID, 0,
IntegrationTestHelper.STRING_RESOURCE_INSTANCE_ID, values, Type.STRING));
// Verify Write result
assertEquals(ResponseCode.CHANGED, response.getCode());
assertNotNull(response.getCoapResponse());
assertThat(response.getCoapResponse(), is(instanceOf(Response.class)));
// read multi instance
ReadResponse readResponse = helper.server.send(helper.getCurrentRegistration(), new ReadRequest(contentFormat,
IntegrationTestHelper.TEST_OBJECT_ID, 0, IntegrationTestHelper.STRING_RESOURCE_INSTANCE_ID));
// verify result
assertEquals(CONTENT, readResponse.getCode());
assertContentFormat(contentFormat, readResponse);
LwM2mMultipleResource resourceInstance = (LwM2mMultipleResource) readResponse.getContent();
assertEquals("value10", resourceInstance.getInstance(10).getValue());
assertEquals("value20", resourceInstance.getInstance(20).getValue());
} and it works for me. SenMLCBOR payload in the response of Read Request looks like : 82a300623230036776616c75653230216b2f323030302f302f31302fa200623130036776616c75653130 Maybe you could share your payload or a wireshark capture this way I could look at this ? |
Regarding .1, Here is the SenML CBOR for my write request: Regarding .2, I think you are correct, it looks like iPATCH is the correct method. |
Good we agree on that 🙂 Regarding .1, I'm lost as I'm not able to reproduce this... So please give me some input that I could understand. |
Sorry for the late reply... Yes, I'm using the Leshan client, I'm using the most current code on master (SHA 7c5a398). I repeated the test using the leshan client and leshan server on the master branch, I have a custom object with a multi-resource string using indexes 10/20, attached the object here. When I use the leshan server, set the encoding to SENML_CBOR and try to read the MV stirng resource, I get the following error: 2021-06-17 17:14:20,339 ERROR LwM2mCoapResource - Exception while handling request [CON-GET MID=30688, Token=C0209FF39E11B708, OptionSet={"Uri-Path":["666","0","1002"], "Accept":"application/senml+cbor"}, no payload] on the resource /666 from Identity /127.0.0.1:5783[unsecure] You can see that the read request is object 666, instance 0, resource 1002, but leshan is interpreting the resource instance values as 100210 and 100220, which is incorrect. |
I will try this. Thx for this clarification. |
OK I succeed to reproduce it. 🎊 I don't know if you just missed to share with me the model of your 666 object, but keep in mind that if you don't use a model for your object (at client and server side) you will probably face some issue |
Sorry, here is the XML file |
This bug affected SenML CBOR and SenML JSON for multi instance resource where resource id and or resource instance id is pretty long.
Tested the PR and it looks good on my side now. Thanks! |
Thx to you for taking time to reporting this 🙏 ! This helps a lot. (This is now integrated in master) |
Hello,
I apologize but I do not have the resources to submit a pull request, but there are 2 bugs in the LWM2M 1.1 implementation that need to be fixed:
Define an object with a String resource with multiple values, define 2 values with resource array ids 10 and 20, and try to read them using senml_cbor. You will get an error. The bug is in LwM2mNodeSenMLEncoder. I fixed it in the patch attached, but I'm not sure this is the best way to fix it, please review.
handlePATCH was incorrectly named handleIPatch which caused all composite writes to fail. See attached patch.
Thanks.
leshan_fixes.txt
The text was updated successfully, but these errors were encountered: