Hi,
just looked a little bit closer at your tt_query implementation and was reminded that we had a small discussion in IRC some weeks ago.
<ecsv> can those flags be mixed? <ordex> mixed? in which way? <ecsv> request | table -> still a valid package <ecsv> sry, packet <ordex> yes it is, because it means "I'm requesting a full table" <ordex> all the combinations are valid
This sounds wrong when I look at the tt_query packet. There we have a tt_data field which can used used exclusive by either TT_REQUEST (x)or TT_RESPONSE. Therefore, it is not possible to use both flags at the same time. Can you please explain how this is handled or otherwise change it so that only one bit in flags is used to decide if it is an response or a request.
Maybe this was the result of the discussion with Marek about the roaming stuff - but i don't think that it applies here.
Kind regards, Sven
Hi Sven, hi all,
On Sun, Jun 12, 2011 at 11:52:00AM +0200, Sven Eckelmann wrote:
Hi,
just looked a little bit closer at your tt_query implementation and was reminded that we had a small discussion in IRC some weeks ago.
<ecsv> can those flags be mixed? <ordex> mixed? in which way? <ecsv> request | table -> still a valid package <ecsv> sry, packet <ordex> yes it is, because it means "I'm requesting a full table" <ordex> all the combinations are valid
This sounds wrong when I look at the tt_query packet. There we have a tt_data field which can used used exclusive by either TT_REQUEST (x)or TT_RESPONSE. Therefore, it is not possible to use both flags at the same time. Can you please explain how this is handled or otherwise change it so that only one bit in flags is used to decide if it is an response or a request.
At the beginning we had one bit only with this meaning:
0 => TT_REQUEST 1 => TT_RESPONSE
But then we had a disussion about: 1) what will we do if we want to add more packet types in the future? 2) is it correct or not to use 0x0 as flag?
Regarding 1) I thought that using a "two bits flag" could help in reserving two configurations more for the future (0x0 and 0x3).
Regarding 2) we switched to TT_REQ = 0x01 and TT_RESP = 0x2.
In my opinion, leaving tt_req = 0x0 and tt_resp = 0x1 would be better, but it seemed to be not so correct.
I hope my explanation is clear. Morover I think that someone should explain me if using 0x0 as a meaningful flag is correct or not :P (I know that field & 0x0 will always be false :P)
Maybe this was the result of the discussion with Marek about the roaming stuff
- but i don't think that it applies here.
No, this is not related to the roaming stuff.
Regards,
Antonio Quartulli wrote:
just looked a little bit closer at your tt_query implementation and was reminded that we had a small discussion in IRC some weeks ago.
<ecsv> can those flags be mixed? <ordex> mixed? in which way? <ecsv> request | table -> still a valid package <ecsv> sry, packet <ordex> yes it is, because it means "I'm requesting a full table" <ordex> all the combinations are valid
This sounds wrong when I look at the tt_query packet. There we have a tt_data field which can used used exclusive by either TT_REQUEST (x)or TT_RESPONSE. Therefore, it is not possible to use both flags at the same time. Can you please explain how this is handled or otherwise change it so that only one bit in flags is used to decide if it is an response or a request.
At the beginning we had one bit only with this meaning:
0 => TT_REQUEST 1 => TT_RESPONSE
Could be correct - never followed the discussion in detail. I asked Marek and he said something about roaming.
But then we had a disussion about:
- what will we do if we want to add more packet types in the future?
Depends on the type. For example full-table creates different packet types together with the TT_REQUEST and TT_RESPONSE. But using TT_REQUEST and TT_RESPONSE together doesn't work at all. So forget about the flag and think more about a currently undefined amount of bits which encode the type of packet. Encoding the request and reply in two different bits and call them flags doesn't really make sense.
- is it correct or not to use 0x0 as flag?
A not existing bit position is not really a "flag". The bit at position zero (lsb) could be used as flag to decide if it is a request or a response - but using nothing to store something is... <please insert your favourite swear word here>.
Regarding 1) I thought that using a "two bits flag" could help in reserving two configurations more for the future (0x0 and 0x3).
Then you say that you don't use flags but 2 bits to store 2 values. Please don't use it in your code as flags, but use something like (x & TT_TYPE_MASK) to extract the corresponding bits and compare the resulting values against representation of TT_RESPONSE and TT_RESPONSE (TT_TYPE_MASK would be 0x3, TT_RESPONSE 0 and TT_REQUEST 3). Also remove the TT_RESPONSE and TT_REQUEST from the enum tt_query_flags and use another enum for them.
Kind regards, Sven
Sven Eckelmann wrote:
Then you say that you don't use flags but 2 bits to store 2 values. Please don't use it in your code as flags, but use something like (x & TT_TYPE_MASK) to extract the corresponding bits and compare the resulting values against representation of TT_RESPONSE and TT_RESPONSE (TT_TYPE_MASK would be 0x3, TT_RESPONSE 0 and TT_REQUEST 3). Also remove the TT_RESPONSE and TT_REQUEST from the enum tt_query_flags and use another enum for them.
s/TT_REQUEST 3/TT_REQUEST 2/
1 and 3 are currently undefined.
Kind regards, Sven
On Mon, Jun 13, 2011 at 04:29:16PM +0200, Sven Eckelmann wrote:
Antonio Quartulli wrote:
just looked a little bit closer at your tt_query implementation and was reminded that we had a small discussion in IRC some weeks ago.
<ecsv> can those flags be mixed? <ordex> mixed? in which way? <ecsv> request | table -> still a valid package <ecsv> sry, packet <ordex> yes it is, because it means "I'm requesting a full table" <ordex> all the combinations are valid
This sounds wrong when I look at the tt_query packet. There we have a tt_data field which can used used exclusive by either TT_REQUEST (x)or TT_RESPONSE. Therefore, it is not possible to use both flags at the same time. Can you please explain how this is handled or otherwise change it so that only one bit in flags is used to decide if it is an response or a request.
At the beginning we had one bit only with this meaning:
0 => TT_REQUEST 1 => TT_RESPONSE
Could be correct - never followed the discussion in detail. I asked Marek and he said something about roaming.
We did something similar for the tt_change flag field. I think I have to give a look at it too.
But then we had a disussion about:
- what will we do if we want to add more packet types in the future?
Depends on the type. For example full-table creates different packet types together with the TT_REQUEST and TT_RESPONSE. But using TT_REQUEST and TT_RESPONSE together doesn't work at all. So forget about the flag and think more about a currently undefined amount of bits which encode the type of packet. Encoding the request and reply in two different bits and call them flags doesn't really make sense.
- is it correct or not to use 0x0 as flag?
A not existing bit position is not really a "flag". The bit at position zero (lsb) could be used as flag to decide if it is a request or a response - but using nothing to store something is... <please insert your favourite swear word here>.
Regarding 1) I thought that using a "two bits flag" could help in reserving two configurations more for the future (0x0 and 0x3).
Then you say that you don't use flags but 2 bits to store 2 values. Please don't use it in your code as flags, but use something like (x & TT_TYPE_MASK) to extract the corresponding bits and compare the resulting values against representation of TT_RESPONSE and TT_RESPONSE (TT_TYPE_MASK would be 0x3, TT_RESPONSE 0 and TT_REQUEST 3). Also remove the TT_RESPONSE and TT_REQUEST from the enum tt_query_flags and use another enum for them.
Ok, I agree. I'm going to propose a patch to clean it up. Thanks for all the suggestions.
Regards,
b.a.t.m.a.n@lists.open-mesh.org
-
Antonio Quartulli -
Sven Eckelmann