Skip to content

Commit

Permalink
Correctly process DESCRIBE request. Additional logging
Browse files Browse the repository at this point in the history
  • Loading branch information
@avtolstoy
avtolstoy committed Jan 1, 2017
1 parent b57dffc commit c54ecc3
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 3 deletions.
8 changes: 6 additions & 2 deletions communication/src/protocol.cpp
View file
Expand Up @@ -58,8 +58,11 @@ ProtocolError Protocol::handle_received_message(Message& message,
// 4 bytes header, 1 byte token, 2 bytes location path
// 2 bytes optional single character location path for describe flags
int descriptor_type = DESCRIBE_ALL;
if (message.length()>8)
if (message.length()>8 && queue[8] <= DESCRIBE_ALL) {
descriptor_type = queue[8];
} else if (message.length() > 8) {
LOG(WARN, "Invalid DESCRIBE flags %02x", queue[8]);
}
error = send_description(token, msg_id, descriptor_type);
break;
}
Expand Down Expand Up @@ -466,7 +469,8 @@ ProtocolError Protocol::send_description(token_t token, message_id_t msg_id, int
appender.append('}');
int msglen = appender.next() - (uint8_t*) buf;
message.set_length(msglen);
LOG(INFO,"Sending describe message");
LOG(INFO,"Sending %s%s describe message", desc_flags & DESCRIBE_SYSTEM ? "S" : "",
desc_flags & DESCRIBE_APPLICATION ? "A" : "");
ProtocolError error = channel.send(message);
if (error==NO_ERROR && descriptor.app_state_selector_info)
{
Expand Down
11 changes: 10 additions & 1 deletion communication/src/spark_protocol.cpp
View file
Expand Up @@ -1399,6 +1399,8 @@ bool SparkProtocol::send_description(int description_flags, msg& message)
int desc_len = description(queue + 2, message.token, queue[2], queue[3], description_flags);
queue[0] = (desc_len >> 8) & 0xff;
queue[1] = desc_len & 0xff;
LOG(INFO,"Sending %s%s describe message", description_flags & DESCRIBE_SYSTEM ? "S" : "",
description_flags & DESCRIBE_APPLICATION ? "A" : "");
return blocking_send(queue, desc_len + 2)>=0;
}

Expand All @@ -1408,7 +1410,14 @@ bool SparkProtocol::handle_message(msg& message, token_t token, CoAPMessageType:
{
case CoAPMessageType::DESCRIBE:
{
if (!send_description(DESCRIBE_SYSTEM, message) || !send_description(DESCRIBE_APPLICATION, message)) {
int desc_flags = DESCRIBE_ALL;
if (message.len > 8 && queue[8] <= DESCRIBE_ALL) {
desc_flags = queue[8];
} else if (message.len > 8) {
LOG(WARN, "Invalid DESCRIBE flags %02x", queue[8]);
}

if (!send_description(desc_flags, message)) {
return false;
}
break;
Expand Down

3 comments on commit c54ecc3

@jlkalberer
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@avtolstoy - I had issues with this when trying to get functions/variables because of instead of 1 coap message == 1 response, I had to listen for two responses.

This looks like it should fix things but can I expect that the entire description (system + functions + variables) will fit in the COAP message or should I start passing flags to get both?

@avtolstoy
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we talking UDP or TCP here? Electron receives and processes separate SYSTEM/APPLICATION DESCRIBE requests; I would assume exactly to fit the CoAP message into MTU. With TCP that shouldn't be a problem at all, since TCP is stream based. Photon doesn't receive any flags and defaults to DESCRIBE_ALL. Previously it was responding with two separate messages as described in #1181. There is still a piece of code missing that strips the AES padding, but no-flags + padding should still be correctly handled now.

@jlkalberer
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my case, I am sending a message over COAP with the local cloud with P1/Photon.

I think my code should handle your changes but the prior behavior was very confusing.

Please sign in to comment.