1-wire. Improved comments

3 files changed, 155 insertions, 53 deletions

diff --git a/os/hal/include/onewire.h b/os/hal/include/onewire.h
index a5ebaa4..290a9b3 100644
--- a/os/hal/include/onewire.h
+++ b/os/hal/include/onewire.h
@@ -27,8 +27,6 @@
 #ifndef _ONEWIRE_H_
 #define _ONEWIRE_H_
 
-#include "hal.h" //FIXME: delete this line when integration done
-
 #if HAL_USE_ONEWIRE || defined(__DOXYGEN__)
 
 /*===========================================================================*/
@@ -36,7 +34,7 @@
 /*===========================================================================*/
 /**
  * @brief   Enable synthetic test for 'search ROM' procedure.
- * @note    Only for debugging/testing.
+ * @note    Only for debugging/testing!
  */
 #define ONEWIRE_SYNTH_SEARCH_TEST         FALSE
 
@@ -58,6 +56,14 @@
 /* Derived constants and error checks.                                       */
 /*===========================================================================*/
 
+#if !HAL_USE_PWM
+#error "1-wire Driver requires HAL_USE_PWM"
+#endif
+
+#if !HAL_USE_PAL
+#error "1-wire Driver requires HAL_USE_PAL"
+#endif
+
 /*===========================================================================*/
 /* Driver data structures and types.                                         */
 /*===========================================================================*/
@@ -83,11 +89,11 @@ typedef uint_fast8_t (*onewire_read_bit_t)(void);
  * @brief   Driver state machine possible states.
  */
 typedef enum {
-  ONEWIRE_UNINIT = 0,
-  ONEWIRE_STOP = 1,
-  ONEWIRE_READY = 2,
+  ONEWIRE_UNINIT = 0,         /**< Not initialized.                         */
+  ONEWIRE_STOP = 1,           /**< Stopped.                                 */
+  ONEWIRE_READY = 2,          /**< Ready.                                   */
 #if ONEWIRE_USE_STRONG_PULLUP
-  ONEWIRE_PULL_UP
+  ONEWIRE_PULL_UP             /**< Pull up asserted.                        */
 #endif
 } onewire_state_t;
 
@@ -95,87 +101,177 @@ typedef enum {
  * @brief   Search ROM procedure possible state.
  */
 typedef enum {
-  ONEWIRE_SEARCH_ROM_SUCCESS = 0,
-  ONEWIRE_SEARCH_ROM_LAST,
-  ONEWIRE_SEARCH_ROM_ERROR
+  ONEWIRE_SEARCH_ROM_SUCCESS = 0,   /**< ROM successfully discovered.       */
+  ONEWIRE_SEARCH_ROM_LAST = 1,      /**< Last ROM successfully discovered.  */
+  ONEWIRE_SEARCH_ROM_ERROR = 2      /**< Error happened during search.      */
 } search_rom_result_t;
 
 /**
  * @brief   Search ROM procedure iteration enum.
  */
 typedef enum {
-  ONEWIRE_SEARCH_ROM_FIRST = 0,
-  ONEWIRE_SEARCH_ROM_NEXT
+  ONEWIRE_SEARCH_ROM_FIRST = 0,     /**< First search run.                  */
+  ONEWIRE_SEARCH_ROM_NEXT = 1       /**< Next search run.                   */
 } search_iteration_t;
 
 /**
  * @brief   Driver configuration structure.
  */
 typedef struct {
+  /**
+   * @brief Pointer to @p PWM driver used for communication.
+   */
   PWMDriver                 *pwmd;
+  /**
+   * @brief Number of PWM channel used as master pulse generator.
+   */
   size_t                    master_channel;
+  /**
+   * @brief Number of PWM channel used as sample interrupt generator.
+   */
   size_t                    sample_channel;
+  /**
+   * @brief Pointer to function performing read of single bit.
+   * @note  It must be callable from any context.
+   */
   onewire_read_bit_t        readBitX;
 #if ONEWIRE_USE_STRONG_PULLUP
+  /**
+   * @brief Pointer to function asserting of strong pull up.
+   */
   onewire_pullup_assert_t   pullup_assert;
+  /**
+   * @brief Pointer to function releasing of strong pull up.
+   */
   onewire_pullup_release_t  pullup_release;
 #endif
 } onewireConfig;
 
 /**
- * @brief     Some small variable used in 'search ROM' procedure combined
- *            in single machine word to save RAM.
+ * @brief     Search ROM registry. Contains small variables used
+ *            in 'search ROM' procedure.
  */
 typedef struct {
-  uint32_t      single_device: 1;   /**< @brief Bool flag                   */
-  uint32_t      search_iter: 1;     /**< @brief 0 - first, 1 - next         */
-  uint32_t      result: 2;          /**< @brief 0 - success, 1 - last, 2 - error.*/
-  uint32_t      bit_step: 2;        /**< @brief 0 - direct, 1 - complemented, 2 - generated by master.     */
-  uint32_t      bit_buf: 2;         /**< @brief Acquired bits. 0s - direct, 1st - complement  */
-  uint32_t      rombit: 7;          /**< @brief Currently processing ROM bit. Must be big enough to store number 64.*/
-  uint32_t      devices_found: 8;   /**< @brief Devices count discovered on bus .*/
+  /**
+   * @brief Bool flag. If @p true than only bus has only one slave device.
+   */
+  uint32_t      single_device: 1;
+  /**
+   * @brief Search iteration (@p search_iteration_t enum).
+   */
+  uint32_t      search_iter: 1;
+  /**
+   * @brief Result of discovery procedure (@p search_rom_result_t enum).
+   */
+  uint32_t      result: 2;
+  /**
+   * @brief One of 3 steps of bit discovery.
+   * @details 0 - direct, 1 - complemented, 2 - generated by master.
+   */
+  uint32_t      bit_step: 2;
+  /**
+   * @brief Values acquired during bit discovery.
+   */
+  uint32_t      bit_buf: 2;
+  /**
+   * @brief Currently processing ROM bit.
+   * @note  Must be big enough to store number 64.
+   */
+  uint32_t      rombit: 7;
+  /**
+   * @brief Total device count discovered on bus.
+   * @note  Maximum 256.
+   */
+  uint32_t      devices_found: 8;
 } search_rom_reg_t;
 
 /**
  * @brief     Helper structure for 'search ROM' procedure
  */
 typedef struct {
+  /**
+   * @brief   Search ROM registry.
+   */
   search_rom_reg_t  reg;
-  uint8_t           *retbuf; /* buffer for currently discovering ROM */
+  /**
+   * @brief   Pointer to buffer with currently discovering ROM
+   */
+  uint8_t           *retbuf;
+  /**
+   * @brief   Previously discovered ROM.
+   */
   uint8_t           prev_path[8];
-  int8_t            last_zero_branch; /* negative values uses to point out of tree root */
-  int8_t            prev_zero_branch; /* negative values uses to point out of tree root */
+  /**
+   * @brief   Last zero turn branch.
+   * @note    Negative values use to point out of device tree's root.
+   */
+  int8_t            last_zero_branch;
+  /**
+   * @brief   Previous zero turn branch.
+   * @note    Negative values use to point out of device tree's root.
+   */
+  int8_t            prev_zero_branch;
 } onewire_search_rom_t;
 
 /**
- * @brief     Some small variable used in driver combined
+ * @brief     Onewire registry. Some small variables combined
  *            in single machine word to save RAM.
  */
 typedef struct {
 #if ONEWIRE_USE_STRONG_PULLUP
   /**
-   * @brief     This flag will be asserted by driver to signalize
-   *            ISR part when strong pullup needed.
+   * @brief   This flag will be asserted by driver to signalizes
+   *          ISR part when strong pull up needed.
    */
   uint32_t      need_pullup: 1;
 #endif
-  uint32_t      slave_present: 1;   /**< @brief Bool flag                   */
-  uint32_t      state: 2;           /**< @brief Driver state         */
-  uint32_t      bit: 4;             /**< @brief Bit number in currently receiving byte. Must be big enough to store 8 */
-  uint32_t      final_timeslot: 1;  /**< @brief bool flag for premature timer stop prevention */
-  uint32_t      bytes: 16;          /**< @brief Bytes number to be processing in current transaction.     */
+  /**
+   * @brief   Bool flag. If @p true than at least one device presence on bus.
+   */
+  uint32_t      slave_present: 1;
+  /**
+   * @brief   Driver internal state (@p onewire_state_t enum).
+   */
+  uint32_t      state: 2;
+  /**
+   * @brief   Bit number in currently receiving/sending byte.
+   * @note    Must be big enough to store 8.
+   */
+  uint32_t      bit: 4;
+  /**
+   * @brief   Bool flag for premature timer stop prevention.
+   */
+  uint32_t      final_timeslot: 1;
+  /**
+   * @brief   Bytes number to be processing in current transaction.
+   */
+  uint32_t      bytes: 16;
 } onewire_reg_t;
 
 /**
- * @brief   Structure representing an 1-wire driver.
+ * @brief     Structure representing an 1-wire driver.
  */
 typedef struct {
+  /**
+   * @brief   Onewire registry.
+   */
   onewire_reg_t         reg;
+  /**
+   * @brief   Onewire config.
+   */
   const onewireConfig   *config;
+  /**
+   * @brief   Config for underlying PWM driver.
+   */
   PWMConfig             pwmcfg;
+  /**
+   * @brief   Pointer to I/O data buffer.
+   */
   uint8_t               *buf;
+  /**
+   * @brief   Search ROM helper structure.
+   */
   onewire_search_rom_t  search_rom;
-
   /**
    * @brief   Thread waiting for I/O completion.
    */
diff --git a/os/hal/src/onewire.c b/os/hal/src/onewire.c
index 0717893..825bb63 100644
--- a/os/hal/src/onewire.c
+++ b/os/hal/src/onewire.c
@@ -18,19 +18,19 @@
 /* Main ideas:                                                               */
 /*===========================================================================
 
-1) switch PWM output pin it opendrain mode.
-2) start 2 channels simultaneously. First (master channel) generates
+1) switch PWM output pin it open drain mode.
+2) start 2 channels _simultaneously_. First (master channel) generates
    pulses (read time slots) second (sample channel) generates interrupts
    from where read pin function calls.
 
--        ---------------------------------------------- master channel
- |     /
-  ----
--                  ------------------------------------ sample channel
- |                 |
-  ------------------
-                   ^
-                   read interrupt here
+-      --------------------------------------- master channel generates pulses
+ |   /
+  --
+-                ----------------------------- sample channel reads pad state
+ |               |
+  ----------------
+                 ^
+                 | read interrupt fires here
 
 For data write it is only master channel needed. Data bit width updates
 on every timer overflow event.
@@ -39,8 +39,6 @@ on every timer overflow event.
 /*===========================================================================*/
 /* General recommendations for strong pull usage                             */
 /*===========================================================================
- *
- *
  * 1) Use separate power rail instead of strong pull up whenever possible.
  *    Driver's strong pull up feature is very interrupt jitter sensible.
  * 2) Use special 1-wire bus master (DS2484 for example) if you are
@@ -101,7 +99,7 @@ onewireDriver OWD1;
 /* Driver local variables and types.                                         */
 /*===========================================================================*/
 /**
- * @brief     Config for fast intialization of all fields
+ * @brief     Config for fast initialization of all fields
  */
 static const PWMConfig pwm_default_cfg = {
   1000000,
@@ -837,7 +835,7 @@ size_t onewireSearchRom(onewireDriver *owp, uint8_t *result,
 }
 
 /*
- * Included (if enabled) test code
+ * Include test code (if enabled).
  */
 #if ONEWIRE_SYNTH_SEARCH_TEST
 #include "search_rom_synth.c"
diff --git a/testhal/STM32/STM32F4xx/onewire/onewire_test.c b/testhal/STM32/STM32F4xx/onewire/onewire_test.c
index 2f0d047..0f8e84b 100644
--- a/testhal/STM32/STM32F4xx/onewire/onewire_test.c
+++ b/testhal/STM32/STM32F4xx/onewire/onewire_test.c
@@ -16,13 +16,20 @@
 
 #include <string.h>
 
-#include "onewire.h"
+#include "hal.h"
 
 /*
  ******************************************************************************
  * DEFINES
  ******************************************************************************
  */
+
+#if defined(BOARD_ST_STM32F4_DISCOVERY)
+#if ONEWIRE_USE_STRONG_PULLUP
+#error "F4 Discovery board has not enough voltage for this feature"
+#endif
+#endif
+
 #define ONEWIRE_MASTER_CHANNEL        2 /* this PWM channel drives bus */
 #define ONEWIRE_SAMPLE_CHANNEL        3 /* this one generates interrupts when sampling needed */
 
@@ -33,8 +40,8 @@
 #else
 #define GPIOB_ONEWIRE                 GPIOB_TACHOMETER
 #include "pads.h"
-#define search_led_on     red_led_on
-#define search_led_off    red_led_off
+#define search_led_on                 red_led_on
+#define search_led_off                red_led_off
 #endif
 
 /*
@@ -48,9 +55,10 @@
  * PROTOTYPES
  ******************************************************************************
  */
-
+/*
+ * Forward declarations
+ */
 static uint_fast8_t onewire_read_bit_X(void);
-
 #if ONEWIRE_USE_STRONG_PULLUP
 static void strong_pullup_assert(void);
 static void strong_pullup_release(void);