addressing PR review: added documentation, moved code and renamed vars

Author: paul-szczepanek-arm

features/FEATURE_BLE/ble/BLETypes.h

@@ -120,9 +120,10 @@ static inline attribute_handle_range_t attribute_handle_range(
 }
 
 /**
- * Type that describes link's encryption state..
+ * Type that describes link's encryption state.
  */
 struct link_encryption_t : SafeEnum<link_encryption_t, uint8_t> {
+    /** struct scoped enum wrapped by the class */
     enum type {
         NOT_ENCRYPTED,          /**< The link is not secured. */
         ENCRYPTION_IN_PROGRESS, /**< Link security is being established. */
@@ -140,6 +141,7 @@ struct link_encryption_t : SafeEnum<link_encryption_t, uint8_t> {
  * Type that describe a pairing failure.
  */
 struct pairing_failure_t : SafeEnum<pairing_failure_t, uint8_t> {
+    /** struct scoped enum wrapped by the class */
     enum type {
         PASSKEY_ENTRY_FAILED = 0x01,
         OOB_NOT_AVAILABLE = 0x02,
@@ -259,12 +261,12 @@ class PasskeyAscii {
     uint8_t ascii[PASSKEY_LEN];
 };
 
-template <size_t octet_size>
-struct octet_type_t {
+template <size_t array_size>
+struct byte_array_t {
     /**
      * Default to all zeroes
      */
-    octet_type_t() {
+    byte_array_t() {
         memset(_value, 0x00, sizeof(_value));
     }
 
@@ -273,7 +275,7 @@ struct octet_type_t {
      *
      * @param input_value value of the data.
      */
-    octet_type_t(const uint8_t *input_value) {
+    byte_array_t(const uint8_t *input_value) {
         memcpy(_value, input_value, sizeof(_value));
     }
 
@@ -283,21 +285,21 @@ struct octet_type_t {
      * @param input_value pointer to buffer.
      * @param size buffer size
      */
-    octet_type_t(const uint8_t* input_value, size_t size) {
+    byte_array_t(const uint8_t* input_value, size_t size) {
         memcpy(_value, input_value, size);
     }
 
     /**
      * Equal operator between two octet types.
      */
-    friend bool operator==(const octet_type_t& lhs, const octet_type_t& rhs) {
+    friend bool operator==(const byte_array_t& lhs, const byte_array_t& rhs) {
         return memcmp(lhs._value, rhs._value, sizeof(lhs._value)) == 0;
     }
 
     /**
      * Non equal operator between two octet types.
      */
-    friend bool operator!=(const octet_type_t& lhs, const octet_type_t& rhs) {
+    friend bool operator!=(const byte_array_t& lhs, const byte_array_t& rhs) {
         return !(lhs == rhs);
     }
 
@@ -326,36 +328,40 @@ struct octet_type_t {
      * Size in byte of a data.
      */
     static size_t size() {
-        return octet_size;
+        return array_size;
     }
 
 protected:
-    uint8_t _value[octet_size];
+    uint8_t _value[array_size];
 };
 
 /** 128 bit keys used by paired devices */
-typedef octet_type_t<16> irk_t;
-typedef octet_type_t<16> csrk_t;
-typedef octet_type_t<16> ltk_t;
+typedef byte_array_t<16> irk_t;
+typedef byte_array_t<16> csrk_t;
+typedef byte_array_t<16> ltk_t;
 
 /** Used to identify LTK for legacy pairing connections */
-typedef octet_type_t<2> ediv_t;
-typedef octet_type_t<8> rand_t;
+typedef byte_array_t<2> ediv_t;
+typedef byte_array_t<8> rand_t;
 
 /** Out of band data exchanged during pairing */
-typedef octet_type_t<16> oob_tk_t; /**< legacy pairing TK */
-typedef octet_type_t<16> oob_lesc_value_t; /**< secure connections oob random 128 value */
-typedef octet_type_t<16> oob_confirm_t; /**< secure connections oob confirmation value */
+typedef byte_array_t<16> oob_tk_t; /**< legacy pairing TK */
+typedef byte_array_t<16> oob_lesc_value_t; /**< secure connections oob random 128 value */
+typedef byte_array_t<16> oob_confirm_t; /**< secure connections oob confirmation value */
 
 /** data to be encrypted */
-typedef octet_type_t<16> encryption_block_t;
-typedef octet_type_t<32> public_key_coord_t;
-typedef octet_type_t<32> dhkey_t;
+typedef byte_array_t<16> encryption_block_t;
+
+/** public key coordinate, two of which define the public key */
+typedef byte_array_t<32> public_key_coord_t;
+
+/** Diffie-Hellman key */
+typedef byte_array_t<32> dhkey_t;
 
 /**
  * MAC address data type.
  */
-struct address_t : public octet_type_t<6> {
+struct address_t : public byte_array_t<6> {
     /**
      * Create an invalid mac address, equal to FF:FF:FF:FF:FF:FF
      */

features/FEATURE_BLE/ble/Gap.h

@@ -2322,7 +2322,7 @@ class Gap {
      * @param[in] handle Handle of the terminated connection.
      * @param[in] reason Reason of the disconnection.
      */
-    virtual void processDisconnectionEvent(Handle_t handle, DisconnectionReason_t reason)
+    void processDisconnectionEvent(Handle_t handle, DisconnectionReason_t reason)
     {
         /* Update Gap state */
         --connectionCount;
@@ -2348,6 +2348,7 @@ class Gap {
      * @param[in] type Advertising type of the packet.
      * @param[in] advertisingDataLen Length of the advertisement data received.
      * @param[in] advertisingData Pointer to the advertisement packet's data.
+     * @param[in] addressType address type of the peer
      */
     void processAdvertisementReport(
         const BLEProtocol::AddressBytes_t peerAddr,

features/FEATURE_BLE/ble/SecurityManager.h

@@ -14,20 +14,18 @@
  * limitations under the License.
  */
 
-#ifndef __SECURITY_MANAGER_H__
-#define __SECURITY_MANAGER_H__
+#ifndef _SECURITY_MANAGER_H_
+#define _SECURITY_MANAGER_H_
 
 #include <stdint.h>
 
 #include "Gap.h"
 #include "CallChainOfFunctionPointersWithContext.h"
 #include "ble/BLETypes.h"
 
-class SecurityManagerEventHandler;
-class LegacySecurityManagerEventHandler;
-
 class SecurityManager {
 public:
+    /** events sent and received when passkey is being entered */
     enum Keypress_t {
         KEYPRESS_STARTED,   /**< Passkey entry started */
         KEYPRESS_ENTERED,   /**< Passkey digit entered */
@@ -36,6 +34,7 @@ class SecurityManager {
         KEYPRESS_COMPLETED, /**< Passkey entry completed */
     };
 
+    /** level of security required from the link by the application */
     enum SecurityMode_t {
         SECURITY_MODE_NO_ACCESS,
         SECURITY_MODE_ENCRYPTION_OPEN_LINK, /**< Require no protection, open link. */
@@ -56,6 +55,7 @@ class SecurityManager {
         ENCRYPTED               /**< The link is secure.*/
     };
 
+    /** Input/output capability of the device and application */
     enum SecurityIOCapabilities_t {
         IO_CAPS_DISPLAY_ONLY = 0x00,     /**< Display only. */
         IO_CAPS_DISPLAY_YESNO = 0x01,    /**< Display and yes/no entry. */
@@ -64,6 +64,7 @@ class SecurityManager {
         IO_CAPS_KEYBOARD_DISPLAY = 0x04, /**< Keyboard and display. */
     };
 
+    /** Result of security requests */
     enum SecurityCompletionStatus_t {
         SEC_STATUS_SUCCESS              = 0x00,  /**< Procedure completed with success. */
         SEC_STATUS_TIMEOUT              = 0x01,  /**< Procedure timed out. */
@@ -103,10 +104,10 @@ class SecurityManager {
     /** The stack will use these functions to signal events to the application,
      *  subclass to override handlers. Use SecurityManager::setSecurityManagerEventHandler
      *  to set the interface implementation to be used. */
-    class SecurityManagerEventHandler {
+    class EventHandler {
     public:
-        SecurityManagerEventHandler() {};
-        virtual ~SecurityManagerEventHandler() {};
+        EventHandler() {};
+        virtual ~EventHandler() {};
 
         ////////////////////////////////////////////////////////////////////////////
         // Pairing
@@ -243,7 +244,6 @@ class SecurityManager {
         /**
          * Indicate that the application needs to send OOB data to the peer.
          *
-         * @param[in] connectionHandle connection connectionHandle
          * @param[in] address address that will be used in the pairing
          * @param[in] random random number used to generate the confirmation
          * @param[in] confirm confirmation value to be use for authentication
@@ -703,7 +703,6 @@ class SecurityManager {
     /**
      * Supply the stack with the OOB data for legacy connections.
      *
-     * @param[in] connectionHandle Handle to identify the connection.
      * @param[in] address address of the peer device this data comes from
      * @param[in] tk pointer to out of band data received containing the temporary key.
      * @return BLE_ERROR_NONE or appropriate error code indicating the failure reason.
@@ -717,7 +716,6 @@ class SecurityManager {
     /**
      * Supply the stack with the OOB data for secure connections.
      *
-     * @param[in] connectionHandle Handle to identify the connection.
      * @param[in] address address of the peer device this data comes from
      * @param[in] random random number used to generate the confirmation
      * @param[in] confirm confirmation value to be use for authentication
@@ -793,7 +791,7 @@ class SecurityManager {
      *
      * @param[in] handler Event Handler interface implementation.
      */
-    virtual void setSecurityManagerEventHandler(SecurityManagerEventHandler* handler) {
+    virtual void setSecurityManagerEventHandler(EventHandler* handler) {
         if (handler) {
             eventHandler = handler;
         } else {
@@ -934,9 +932,9 @@ class SecurityManager {
 private:
     /* Legacy compatibility with old callbacks (from both sides so any
      * combination of new and old works) */
-    class LegacySecurityManagerEventHandler : public SecurityManagerEventHandler {
+    class LegacyEventHandler : public EventHandler {
     public:
-        LegacySecurityManagerEventHandler() :
+        LegacyEventHandler() :
             securitySetupInitiatedCallback(),
             securitySetupCompletedCallback(),
             linkSecuredCallback(),
@@ -980,8 +978,8 @@ class SecurityManager {
     SecurityManagerShutdownCallbackChain_t shutdownCallChain;
 
 protected:
-    SecurityManagerEventHandler*      eventHandler;
-    LegacySecurityManagerEventHandler defaultEventHandler;
+    EventHandler*      eventHandler;
+    LegacyEventHandler defaultEventHandler;
 };
 
-#endif /*__SECURITY_MANAGER_H__*/
+#endif /*_SECURITY_MANAGER_H_*/

features/FEATURE_BLE/ble/generic/GenericGap.h

@@ -254,7 +254,7 @@ class GenericGap : public ::Gap,
     /**
      * @copydoc ::Gap::processConnectionEvent
      */
-    virtual void processConnectionEvent(
+    void processConnectionEvent(
         Handle_t handle,
         Role_t role,
         BLEProtocol::AddressType_t peerAddrType,
@@ -267,17 +267,17 @@ class GenericGap : public ::Gap,
     /**
      * @copydoc ::Gap::processDisconnectionEvent
      */
-    virtual void processDisconnectionEvent(
+    void processDisconnectionEvent(
         Handle_t handle,
         DisconnectionReason_t reason
     );
 
+private:
     /** @note Implements ConnectionEventMonitor.
      *  @copydoc ConnectionEventMonitor::set_connection_event_handler
      */
     void set_connection_event_handler(pal::ConnectionEventHandler *_connection_event_handler);
 
-private:
     void on_scan_timeout();
 
     void process_scan_timeout();