Conversation
src/subsystem/mac/sreq.rs
Outdated
| // This frame's length is 0x1e, instead of 0x26 (as shown in the documentation). Check the link below for more information. | ||
| // https://e2e.ti.com/support/wireless-connectivity/sub-1-ghz-group/sub-1-ghz/f/sub-1-ghz-forum/1578237/launchxl-cc1352r1-wrong-length-for-mac_ws_async_req-command |
There was a problem hiding this comment.
This comment in the code does not make much sense.
It will become false as soon as the code is merged.
There was a problem hiding this comment.
I think this comment is useful to clarify why the value written in our code differs from the value written in the TI-15.4-STACK-CoP Interface Guide.
This document has not changed over the past 9 years (last update: 2016-06-28), so we don't know when they will release a documentation with the info fixed.
There was a problem hiding this comment.
| } | ||
|
|
||
| // AddrNone value was added in order to handle AddressMode zeroed. Check the link below for more information. | ||
| // https://e2e.ti.com/support/wireless-connectivity/sub-1-ghz-group/sub-1-ghz/f/sub-1-ghz-forum/1570804/simplelink-cc13xx-cc26xx-sdk-dstaddrmode-is-zeroed-in-mac_ws_async_ind-packet |
There was a problem hiding this comment.
Why are we adding this comment here?
Either AddrNone is defined in the documentation, in which case this addition does not need to be justified or it is not on the documentation and we should not be adding it.
There was a problem hiding this comment.
The documentation (TI-15.4-STACK-CoP Interface Guide) does not mention the AddressMode with value 0x00 (that's why we don't have it implemented).
Although, the protocol does use this value in a few scenarios. Its use was pointed in the thread linked in the comment. This option "AddrNone" is implemented in the SDK v8.30.01.01 code as well:
| pub struct IeItem { | ||
| pub type_long: bool, | ||
| pub id: u8, | ||
| pub content_len: u16, | ||
| pub content: Vec<u8>, | ||
| } |
There was a problem hiding this comment.
What is this structure representing? We do we need a boolean field called type_long?
There was a problem hiding this comment.
This structure represents an item passed in the IE Payload (used in a few messages). The information passed through the IE Payload is encoded in a specific way, as described by this document.
In order to parse its data, it is useful to have the IeItem structure, similarly as the one implemented in the SDK v8.30.01.01:
Yes, the field type_longis used while parsing its content.
There was a problem hiding this comment.
It seems you are simply mimicking the C structure. Are you sure this is how this would be implemented in Rust? It sounds like there can be 2 types in this structure: either a long or something else. So, it sounds to me this should look something like:
pub enum IeContentType {
SomethingElse(???), // ieTypeLong == False
Long(i32), // ieTypeLong == True
};
pub struct IeItem {
pub id: u8,
pub content_len: u16,
pub content: Vec<IeContentType>,
}There was a problem hiding this comment.
The parsing of this data type is rather dynamic (so it may take a while to really understand and implement a reasonable parser for this) and the TI implementation doesn't cover all the possibilities of parsing it.
Following the specification, first we should parse as a group, creating a list of IeItem:
Then, for each IeItem we should apply the same parser (now as a subgroup) and retrieve the data from the content. It's an iterative process, because we don't know how many subgroups will have in the list until we find one of the subgroups with a specific id indicating that it is the end of the content:
Each subgroup will be parsed in a different way depending of its type (short or long) set by a given bit (and the way we read the other bits change as well, see the diagram below:
Since we have a lot of "branches" being created at each step of the parser and the TI parser does not cover all the possibilities, I opted to leave the IeItem structure here so we can refer to it, but then write a "small" parser in the HAL.
If you prefer, I can move this parser to this repo though.
| pub struct IeItem { | ||
| pub type_long: bool, | ||
| pub id: u8, | ||
| pub content_len: u16, | ||
| pub content: Vec<u8>, | ||
| } |
There was a problem hiding this comment.
Why are we adding this type if this is not being used anywhere?
There was a problem hiding this comment.
Both PAN Advertisement Solicit and PAN Configuration Solicit messages (received as MAC_WS_ASYNC_IND) carry data in the IE Payload field.
So it is being used now.
There was a problem hiding this comment.
Why isn't this type being used anywhere in this Pull Request? Why doesn't it have the serialization and deserialization implemented?
|
This PR is not going to be merged for now. It waits the release of an erratum to the TI documentation |
4 participants
In this PR: