-
Notifications
You must be signed in to change notification settings - Fork 0
Add missing fields to WSAsyncInd #2
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
base: master
Are you sure you want to change the base?
Changes from all commits
c7e425f
b6d2cda
c48e178
3c00ef2
192cb64
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -207,8 +207,11 @@ impl Status { | |
| } | ||
| } | ||
|
|
||
| // 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 | ||
| #[derive(Debug, FromPrimitive, PartialEq, Copy, Clone)] | ||
| pub enum AddressMode { | ||
| AddrNone = 0x00, | ||
| Addr16Bit = 0x02, | ||
| Addr64Bit = 0x03, | ||
| } | ||
|
|
@@ -263,6 +266,7 @@ impl ExtendedAddress { | |
|
|
||
| #[derive(Debug, PartialEq, Copy, Clone)] | ||
| pub enum Address { | ||
| AddrNone, | ||
| Addr16Bit(ShortAddress), | ||
| Addr64Bit(ExtendedAddress), | ||
| } | ||
|
|
@@ -272,11 +276,15 @@ impl Address { | |
| let address_mode = AddressMode::try_decode(Read::by_ref(cursor))?; | ||
|
|
||
| let address = match address_mode { | ||
| AddressMode::AddrNone => { | ||
| std::io::BufRead::consume(cursor, 8); | ||
| Address::AddrNone | ||
| }, | ||
| AddressMode::Addr16Bit => { | ||
| let address = Address::Addr16Bit(ShortAddress::try_decode(cursor)?); | ||
| std::io::BufRead::consume(cursor, 6); | ||
| address | ||
| } | ||
| }, | ||
| AddressMode::Addr64Bit => Address::Addr64Bit(ExtendedAddress::try_decode(cursor)?), | ||
| }; | ||
|
|
||
|
|
@@ -285,6 +293,10 @@ impl Address { | |
|
|
||
| pub fn encode_into(&self, buffer: &mut Vec<u8>) { | ||
| match self { | ||
| Address::AddrNone => { | ||
| buffer.put_u8(AddressMode::AddrNone as u8); | ||
| buffer.extend_from_slice(&[0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]); | ||
| } | ||
| Address::Addr16Bit(address) => { | ||
| buffer.put_u8(AddressMode::Addr16Bit as u8); | ||
| address.encode_into(buffer); | ||
|
|
@@ -298,6 +310,15 @@ impl Address { | |
| } | ||
| } | ||
|
|
||
| // Struct based on the ApiMac_payloadIeItem_t retrieved from | ||
| // the api_mac.h file in the SIMPLELINK-LOWPOWER-F2-SDK v8.30.01.01 | ||
| pub struct IeItem { | ||
| pub type_long: bool, | ||
| pub id: u8, | ||
| pub content_len: u16, | ||
| pub content: Vec<u8>, | ||
| } | ||
|
Comment on lines
+315
to
+320
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is this structure representing? We do we need a boolean field called
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
Yes, the field There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 pub enum IeContentType {
SomethingElse(???), // ieTypeLong == False
Long(i32), // ieTypeLong == True
};
pub struct IeItem {
pub id: u8,
pub content_len: u16,
pub content: Vec<IeContentType>,
}
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Comment on lines
+315
to
+320
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are we adding this type if this is not being used anywhere?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why isn't this type being used anywhere in this Pull Request? Why doesn't it have the serialization and deserialization implemented?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll keep the discussion on this comment here. |
||
|
|
||
| bitflags! { | ||
| #[derive(Debug, Clone)] | ||
| pub struct TxOption: u8 { | ||
|
|
@@ -513,6 +534,21 @@ pub enum MACPIBAttributeId { | |
| ChannelPage = 0xE7, | ||
| PhyCurrentDescriptorId = 0xE8, | ||
| FCSType = 0xE9, | ||
| DiagRxCrcPass = 0xEA, | ||
| DiagRxCrcFail = 0xEB, | ||
| DiagRxBC = 0xEC, | ||
| DiagTxBC = 0xED, | ||
| DiagRxUC = 0xEE, | ||
| DiagTxUC = 0xEF, | ||
| DiagTxUCRetry = 0xF0, | ||
| DiagTxUCFail = 0xF1, | ||
| DiagRxSecureFail = 0xF2, | ||
| DiagTxSecureFail = 0xF3, | ||
| RssiThreshold = 0xF4, | ||
| RangeExtender = 0xF5, | ||
| EnDataAckPending = 0xF6, | ||
| RfFreq = 0xF7, | ||
| PaType = 0xF8, | ||
| } | ||
|
|
||
| impl MACPIBAttributeId { | ||
|
|
@@ -554,6 +590,10 @@ pub enum FHPIBAttributeId { | |
| GTK2Hash = 0x2017, | ||
| GTK3Hash = 0x2018, | ||
| NeighborValidTime = 0x2019, | ||
| CsmaBaseBacoff = 0x201A, | ||
| NumNonSleepDevice = 0x201B, | ||
| NumSleepDevice = 0x201C, | ||
| NumTempTableNode = 0x201D, | ||
| } | ||
|
|
||
| impl FHPIBAttributeId { | ||
|
|
||




There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we adding this comment here?
Either
AddrNoneis 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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: