parsing binary vs hex data

[thedinuka]

When parsing m_nData in function fromJson() the lenght of cTmp is checked to figure out whether the data was sent as binary or hex. I'm a bit worried about hard coding the threshold value for this decision (ie number 12). Is there anything we can do about this?

[mylifeofagony]

Hello Dinuka,

When parsing m_nData in function fromJson() the lenght of cTmp is checked

to figure out whether the data was sent as binary or hex. I'm a bit worried about hard coding the threshold value for this decision (ie number 12). Is there anything we can do about this?

binary was only added to make the code compatible with the old firmware v1.2. As this is working properly now, we can drop all the stuff that won't be needed anymore. So, binary representation should be removed completely.

I had a chat with Pete and James (who is going to handle the Node client

devs) Friday, and there we agreed to revise the protocol a bit, considering a suggestion made by you some time back. Basically we decided not to use JSON in the DA field as discussed previously. Instead, the encoding, pulse width and payload will be concatenated to a single hex string and will be put in the DA field. So the DA field should look like | one byte encoding | 1 reserved byte | 4 byte pulse width | variable number of bytes data |

OK, sounds good.

In any case, we still need to keep track of the encoding and pulse width

for each 433 device. When decoding RF packets, this information can simply be added to the m_nData without any modifications to the rest of the code. So I can go ahead and implement that. When receiving data from the Node, I think it's best to add two members to the Json packet (encoding and pulse width), so that when the encoding is done in the onBoardManager::handle(), I can chose the correct encoder to use based on those members. Let me know what you think.

I had a look at how you implemented the header and was wondering why you chose a ull for that? That's 8 bytes, and the header is only 4 bytes. But I would suggest to have seperate fields for the different values in the class anyway. Makes the code a lot more understandable. I changed and committed the code. Let me know if you like it...

Cheers, Mike

[mylifeofagony]

I just realized that I can't test the code right now, so I will commit it later when I am home.

I will also add parsing functionality for the new header and remove the binary stuff. That means that the code won't work with the old Node client anymore...

Mike

On Mon, Mar 25, 2013 at 12:18 PM, Mike van Dyke mylifeofagony@gmail.comwrote:

Hello Dinuka,

When parsing m_nData in function fromJson() the lenght of cTmp is checked

to figure out whether the data was sent as binary or hex. I'm a bit worried about hard coding the threshold value for this decision (ie number 12). Is there anything we can do about this?

binary was only added to make the code compatible with the old firmware v1.2. As this is working properly now, we can drop all the stuff that won't be needed anymore. So, binary representation should be removed completely.

I had a chat with Pete and James (who is going to handle the Node client

devs) Friday, and there we agreed to revise the protocol a bit, considering a suggestion made by you some time back. Basically we decided not to use JSON in the DA field as discussed previously. Instead, the encoding, pulse width and payload will be concatenated to a single hex string and will be put in the DA field. So the DA field should look like | one byte encoding | 1 reserved byte | 4 byte pulse width | variable number of bytes data |

OK, sounds good.

In any case, we still need to keep track of the encoding and pulse width

for each 433 device. When decoding RF packets, this information can simply be added to the m_nData without any modifications to the rest of the code. So I can go ahead and implement that. When receiving data from the Node, I think it's best to add two members to the Json packet (encoding and pulse width), so that when the encoding is done in the onBoardManager::handle(), I can chose the correct encoder to use based on those members. Let me know what you think.

I had a look at how you implemented the header and was wondering why you chose a ull for that? That's 8 bytes, and the header is only 4 bytes. But I would suggest to have seperate fields for the different values in the class anyway. Makes the code a lot more understandable. I changed and committed the code. Let me know if you like it...

Cheers, Mike