doc: fix WARNING: Invalid definition" due to unamed structs/unions

Fix with a workaround in unnamed unions / structs in bluetooth, i2c,
sensor and uart.

Current documentation parsers (sphinx under Doxygen) don't seem to
understand well unnamed structs / unions. They will not generate any
documentation for any documented members (see left side of
http://imgur.com/mcpBXWc).

A workaround is to make the parser think there is something like a
struct/union/enum name that is actually something with no effect to
the compiler.

Naming it with __unnamed_workaround__ ensures it is clear it is a
workaround while we wait for a final fix. It is #defined to be a NO-OP
to the compiler and rearrange the member documentation as *@param* so
we have some documentation that the non-worked around code fails to
document.

Anonymous structs/union that declare a variable are just given an
internal name.

Workarounds documented in the contribution guidelines.

Change-Id: I4d32cf444f3c5e7d2fb11581e4b41f80e93c9786
Signed-off-by: Inaky Perez-Gonzalez <inaky.perez-gonzalez@intel.com>

doc/contribute/doxygen/structs.rst

@@ -27,6 +27,143 @@ Structs have a simplified template:
 Doxygen does not require any commands to recognize the different comments.
 It does, however, require that line 8 be left blank.
 
+Unnamed structures or unions
+****************************
+
+The *Sphinx* parser seems to get confused by unnamed structures, used
+specially in nested unions / structs:
+
+.. code-block:: c
+
+  struct sensor_value {
+          enum sensor_value_type type;
+          union {
+                  struct {
+                          int32_t val1;
+                          int32_t val2;
+                  };
+                  double dval;
+          };
+  };
+
+This will likely generate an error such as::
+
+  doc/api/io_interfaces.rst:14: WARNING: Invalid definition: Expected identifier in nested name. [error at 0]
+
+    ^
+  doc/api/io_interfaces.rst:14: WARNING: Invalid definition: Expected identifier in nested name. [error at 0]
+
+    ^
+  doc/api/io_interfaces.rst:14: WARNING: Invalid definition: Expected end of definition. [error at 12]
+    sensor_value.__unnamed__
+    ------------^
+
+A workaround is to introduce something that looks like a name to the
+parser but it is a no-op to the compiler:
+
+.. code-block:: c
+
+  /**
+   * Workaround for documentation parser limitations
+   *
+   * Current documentation parsers (sphinx under Doxygen) don't seem to
+   * understand well unnamed structs / unions. A workaround is to make
+   * the parser think there is something like a struct/union/enum name
+   * that is actually something with no effect to the compiler.
+   *
+   * Current choice is to give it a 1B alignment. This basically tells
+   * the compiler to do what is doing now: align it wherever it thinks
+   * it should, as a 1B alignment "restriction" fits any other alignment
+   * restriction we might have.
+   */
+  #define __unnamed_workaround__ __attribute__ ((__aligned__ (1)))
+
+And then use it such as:
+
+.. code-block:: c
+
+  struct sensor_value {
+          enum sensor_value_type type;
+          union __unnamed_workaround__ {
+                  struct __unnamed_workaround__  {
+                          int32_t val1;
+                          int32_t val2;
+                  };
+                  double dval;
+          };
+  };
+
+This is currently defined in :file:`include/toolchain.h`. The issue
+reported to developers in
+https://github.com/sphinx-doc/sphinx/issues/2683.
+
+When running into this issue, the member documentation has to be done
+with *@param* indicators, otherwise they won't be extracted:
+
+.. code-block:: c
+
+  /**
+   * @brief UART device configuration.
+   *
+   * @param port Base port number
+   * @param base Memory mapped base address
+   * @param regs Register address
+   * @param sys_clk_freq System clock frequency in Hz
+   */
+  struct uart_device_config {
+        union __unnamed_workaround__ {
+                uint32_t port;
+                uint8_t *base;
+                uint32_t regs;
+        };
+
+        uint32_t sys_clk_freq;
+  ...
+
+
+Unnamed structures or unions which declare a variable
+-----------------------------------------------------
+
+A special case of this is non-anynoymous unnamed structs/unions, where
+a variable is defined. In this case, the workaround is much more
+simple. Consider:
+
+.. code-block:: c
+
+   union dev_config {
+           uint32_t raw;
+	   struct {
+                   uint32_t        use_10_bit_addr : 1;
+		   uint32_t        speed : 3;
+		   uint32_t        is_master_device : 1;
+		   uint32_t        is_slave_read : 1;
+		   uint32_t        reserved : 26;
+	   } bits;
+   };
+
+This will likely generate an error such as::
+
+  doc/api/io_interfaces.rst:28: WARNING: Invalid definition: Expected identifier in nested name. [error at 19]
+  struct dev_config::@61  dev_config::bits
+  -------------------^
+
+The simple solution is to name that unnamed struct, with an internal
+name (such as *__bits*):
+
+.. code-block:: c
+
+   union dev_config {
+           uint32_t raw;
+	   struct __bits {
+                   uint32_t        use_10_bit_addr : 1;
+		   uint32_t        speed : 3;
+		   uint32_t        is_master_device : 1;
+		   uint32_t        is_slave_read : 1;
+		   uint32_t        reserved : 26;
+	   } bits;
+   };
+
+
 Structure Documentation Example
 *******************************
 
@@ -56,4 +193,4 @@ Incorrect:
 
 The struct has no documentation. Developers that want to expand its
 functionality have no way of understanding why the struct is defined
-this way, nor what its components are.
+this way, nor what its components are.

include/bluetooth/conn.h

@@ -135,19 +135,22 @@ enum {
 	BT_CONN_ROLE_SLAVE,
 };
 
-/** Connection Info Structure */
+/** @brief Connection Info Structure
+ *
+ *
+ *  @param type Connection Type
+ *  @param role Connection Role
+ *  @param le LE Connection specific Info
+ *  @param br BR/EDR Connection specific Info
+ */
 struct bt_conn_info {
-	/** Connection Type */
 	uint8_t type;
 
-	/** Connection Role */
 	uint8_t role;
 
-	union {
-		/** LE Connection specific Info */
+	union __unnamed_workaround__ {
 		struct bt_conn_le_info le;
 
-		/** BR/EDR Connection specific Info */
 		struct bt_conn_br_info br;
 	};
 };

include/bluetooth/gatt.h

@@ -821,24 +821,23 @@ typedef uint8_t (*bt_gatt_read_func_t)(struct bt_conn *conn, uint8_t err,
 				       struct bt_gatt_read_params *params,
 				       const void *data, uint16_t length);
 
-/** @brief GATT Read parameters */
+/** @brief GATT Read parameters
+ *  @param func Read attribute callback
+ *  @param handle_count If equals to 1 single.handle and single.offset
+ *                      are used.  If >1 Read Multiple Characteristic
+ *                      Values is performed and handles are used.
+ *  @param handle Attribute handle
+ *  @param offset Attribute data offset
+ *  @param handles Handles to read in Read Multiple Characteristic Values
+ */
 struct bt_gatt_read_params {
-	/** Read attribute callback */
 	bt_gatt_read_func_t func;
-	/** Handles count.
-	 * If equals to 1 single.handle and single.offset are used.
-	 * If >1 Read Multiple Characteristic Values is performed and handles
-	 * are used.
-	 */
 	size_t handle_count;
-	union {
-		struct {
-			/** Attribute handle */
+	union __unnamed_workaround__ {
+		struct __single {
 			uint16_t handle;
-			/** Attribute data offset */
 			uint16_t offset;
 		} single;
-		/** Handles to read in Read Multiple Characteristic Values */
 		uint16_t *handles;
 	};
 };

include/i2c.h

@@ -107,7 +107,7 @@ struct i2c_msg {
 
 union dev_config {
 	uint32_t raw;
-	struct {
+	struct __bits {
 		uint32_t        use_10_bit_addr : 1;
 		uint32_t        speed : 3;
 		uint32_t        is_master_device : 1;

include/sensor.h

@@ -63,8 +63,8 @@ enum sensor_value_type {
  */
 struct sensor_value {
 	enum sensor_value_type type;
-	union {
-		struct {
+	union __unnamed_workaround__  {
+		struct __unnamed_workaround__ {
 			int32_t val1;
 			int32_t val2;
 		};

include/toolchain.h

@@ -31,4 +31,19 @@
 #include <toolchain/other.h>
 #endif
 
+/**
+ * Workaround for documentation parser limitations
+ *
+ * Current documentation parsers (sphinx under Doxygen) don't seem to
+ * understand well unnamed structs / unions. A workaround is to make
+ * the parser think there is something like a struct/union/enum name
+ * that is actually something with no effect to the compiler.
+ *
+ * Current choice is to give it a 1B alignment. This basically tells
+ * the compiler to do what is doing now: align it wherever it thinks
+ * it should, as a 1B alignment "restriction" fits any other alignment
+ * restriction we might have.
+ */
+#define __unnamed_workaround__ __attribute__((__aligned__(1)))
+
 #endif /* _TOOLCHAIN_H */

include/uart.h

@@ -88,20 +88,21 @@ typedef void (*uart_irq_callback_t)(struct device *port);
  */
 typedef void (*uart_irq_config_func_t)(struct device *port);
 
-/** @brief UART device configuration.*/
+/**
+ * @brief UART device configuration.
+ *
+ * @param port Base port number
+ * @param base Memory mapped base address
+ * @param regs Register address
+ * @param sys_clk_freq System clock frequency in Hz
+ */
 struct uart_device_config {
-	/**
-	 * Base port number
-	 * or memory mapped base address
-	 * or register address
-	 */
-	union {
+	union __unnamed_workaround__ {
 		uint32_t port;
 		uint8_t *base;
 		uint32_t regs;
 	};
 
-/** System clock frequency in Hz.*/
 	uint32_t sys_clk_freq;
 
 #ifdef CONFIG_PCI