Extract Adress related types from Gap.h into BLEProtocol.h #142
Conversation
| @@ -239,7 +239,7 @@ class BLE | |||
| * ble.setAddress(...) should be replaced with | |||
| * ble.gap().setAddress(...). | |||
| */ | |||
| ble_error_t setAddress(Gap::AddressType_t type, const Gap::Address_t address) { | |||
| ble_error_t setAddress(BLEProtocol::AddressType::Type type, const Gap::Address_t address) { | |||
There was a problem hiding this comment.
Why Gap::Address_t is still used instead of BLEProtocol::Address_t ?
|
What other data type should live want inside the BLEProtocol ? |
| * typedef AddressType_t AliasedType_t; | ||
| * would allow the use of AliasedType_t::PUBLIC in code. | ||
| */ | ||
| enum Type { |
There was a problem hiding this comment.
Maybe it can be useful to add a typedef to this type inside BLEProtocol namespace: typedef AddressType::Type AddressType_t. It would allow user code to use address type like this: BLEProtocol::AddressType_t instead of BLEProtocol::AddressType::Type.
|
@pan-: All generic types and data-structures could be migrated to BLEProtocol. I'll soon add datatypes involved in security and white-listing. |
|
@pan- @LiyouZhou @andresag01: please review the latest. |
|
Other opportunities of pulling out abstract protocol-specific types into BLEProtocol.h:
and others to follow. |
|
+1 |
1 similar comment
|
+1 |
|
|
||
| static const unsigned ADDR_LEN = BLEProtocol::ADDR_LEN; /**< Length (in octets) of the BLE MAC address. */ | ||
| typedef BLEProtocol::Address_t Address_t; /**< 48-bit address, LSB format. @Note: Deprecated. Use BLEProtocol::Address_t instead. */ |
There was a problem hiding this comment.
If Address_t is being deprecated, shouldn't all occurrences in this file be replaced?
There was a problem hiding this comment.
It's being deprecated, not removed. If it was removed now, it will break user code.
There was a problem hiding this comment.
I know what the word means, but shouldn't we be leading by example and at least make the changes inside the same file?
When I see this it looks weird:
const Address_t peerAddr,
BLEProtocol::AddressType_t ownAddrType,
There was a problem hiding this comment.
I might be missing something but it is already the case, no ?
There was a problem hiding this comment.
@pan- there were a couple of places in Gap.h where I had missed replacing Gap::Address_t with BLEProtocol::Address_t
|
+1 |
|
@marcuschangarm thanks for pointing out the use of deprecated Gap::Address_t. I've fixed this now. Please review. |
|
Looks good. What are your future plans for BLEProtocol? |
|
@marcuschangarm I believe we will increasingly move protocol specific constants and algorithms into this file. For instance, I'll soon move types supporting whitelisting into BLEProtocol.h. |
Extract Adress related types from Gap.h into BLEProtocol.h
|
published latest version: 2.1.15 to the private registry. |
While adding support for white-listing, I realized that a lot of core types, such as Address_t, are currently residing in Gap.h but are more abstract than Gap. They should be abstracted out of Gap otherwise they will result in circular dependencies.
I've also taken the opportunity to cleanup the types and add some documentation.
internal-ref: IOTSFW-1416
@pan- @marcuschangarm @andresag01 @LiyouZhou