Unusual code comments using doxygen format

[atsju]

I had a look at the documentation of the code. Here is a first example in Utilities/sequencer. The doxygen comments in .h for the function are just excellent. But then there is some comments in the .c file. Which is fine but is using doxygen syntax (/** doc */) so it gets integrated to the documentation. Maybe here it is normal and you want to have full documentation in doxygen but it's at least unusual. To be more explicit /** doc */ should be replaced by /* doc */ to not be processed by doxygen.

A second example that makes me think this is not the expected result is UTIL_LPM_GetMode function in Utilities\lpm\tiny_lpm\stm32_lpm.c. There is doxygen documentation inside the code that is clearly not intended to be doxygenated as it's nonsense out of context.

To conclude: all comments inside functions should be reviewed and should not use doxygen syntax. I have only checked some files but I suppose that there are much more impacted by this style and even other STM32Cube package may be impacted also.

Utilities\sequencer\stm32_seq.c

.h

/**
 * @brief This function requests the sequencer to execute all pending tasks using round robin mechanism.
 *        When no task are pending, it calls UTIL_SEQ_Idle();
 *        This function should be called in a while loop in the application
 *
 * @param Mask_bm list of task (bit mapping) that is be kept in the sequencer list.
 *
 */
void UTIL_SEQ_Run( UTIL_SEQ_bm_t Mask_bm );

.c

/**
 * This function can be nested.
 * That is the reason why many variables that are used only in that function are declared static.
 * Note: These variables could have been declared static in the function.
 */
void UTIL_SEQ_Run( UTIL_SEQ_bm_t Mask_bm )
{
  uint32_t counter;
  UTIL_SEQ_bm_t current_task_set;
  UTIL_SEQ_bm_t super_mask_backup;

  /**
   *  When this function is nested, the mask to be applied cannot be larger than the first call
   *  The mask is always getting smaller and smaller
   *  A copy is made of the mask set by UTIL_SEQ_Run() in case it is called again in the task
   */
  super_mask_backup = SuperMask;
  SuperMask &= Mask_bm;

  /**
   * There are two independent mask to check:
   * TaskMask that comes from UTIL_SEQ_PauseTask() / UTIL_SEQ_ResumeTask
   * SuperMask that comes from UTIL_SEQ_Run
   * If the waited event is there, exit from  UTIL_SEQ_Run() to return to the
   * waiting task
   */
  while(((TaskSet & TaskMask & SuperMask) != 0U) && ((EvtSet & EvtWaited)==0U))
  {
    counter = 0U;
    /**
     * When a flag is set, the associated bit is set in TaskPrio[counter].priority mask depending
     * on the priority parameter given from UTIL_SEQ_SetTask()
     * The while loop is looking for a flag set from the highest priority maskr to the lower
     */
    while((TaskPrio[counter].priority & TaskMask & SuperMask)== 0U)
    {
      counter++;
    }

    current_task_set = TaskPrio[counter].priority & TaskMask & SuperMask;

    /**
     * The round_robin register is a mask of allowed flags to be evaluated.
     * The concept is to make sure that on each round on UTIL_SEQ_Run(), if two same flags are always set,
     * the sequencer does not run always only the first one.
     * When a task has been executed, The flag is removed from the round_robin mask.
     * If on the next UTIL_SEQ_RUN(), the two same flags are set again, the round_robin mask will mask out the first flag
     * so that the second one can be executed.
     * Note that the first flag is not removed from the list of pending task but just masked by the round_robin mask
     *
     * In the check below, the round_robin mask is reitialize in case all pending tasks haven been executed at least once
     */
    if ((TaskPrio[counter].round_robin & current_task_set) == 0U)
    {
      TaskPrio[counter].round_robin = UTIL_SEQ_ALL_BIT_SET;
    }

    /** Read the flag index of the task to be executed
     *  Once the index is read, the associated task will be executed even though a higher priority stack is requested
     *  before task execution.
     */
    CurrentTaskIdx = (SEQ_BitPosition(current_task_set & TaskPrio[counter].round_robin));

    /** remove from the roun_robin mask the task that has been selected to be executed */
    TaskPrio[counter].round_robin &= ~(1U << CurrentTaskIdx);

    UTIL_SEQ_ENTER_CRITICAL_SECTION( );
    /** remove from the list or pending task the one that has been selected to be executed */
    TaskSet &= ~(1U << CurrentTaskIdx);
    /** remove from all priority mask the task that has been selected to be executed */
    for (counter = UTIL_SEQ_CONF_PRIO_NBR; counter != 0U; counter--)
    {
      TaskPrio[counter - 1U].priority &= ~(1U << CurrentTaskIdx);
    }
    UTIL_SEQ_EXIT_CRITICAL_SECTION( );
    /** Execute the task */
    TaskCb[CurrentTaskIdx]( );
  }

rendering

Utilities\lpm\tiny_lpm\stm32_lpm.c

UTIL_LPM_Mode_t UTIL_LPM_GetMode( void )
{
  UTIL_LPM_Mode_t mode_selected;

  UTIL_LPM_ENTER_CRITICAL_SECTION( );

  if( StopModeDisable != UTIL_LPM_NO_BIT_SET )
  {
    /**
     * At least one user disallows Stop Mode
     */
    mode_selected = UTIL_LPM_SLEEPMODE;
  }
  else
  {
    if( OffModeDisable != UTIL_LPM_NO_BIT_SET )
    {
      /**
       * At least one user disallows Off Mode
       */
      mode_selected = UTIL_LPM_STOPMODE;
    }
    else
    {
      mode_selected = UTIL_LPM_OFFMODE;
    }
  }

  UTIL_LPM_EXIT_CRITICAL_SECTION( );

  return mode_selected;
}

[RKOUSTM]

Hi @atsju

Thank you for your contribution. The issue you pointed out has been confirmed. An internal tracker has been logged and a fix will be implemented and made available in the frame of a future release.

Thank you once again for your contribution.

With regards,

[RKOUSTM]

ST Internal Reference: 111672

[ALABSTM]

Hi @atsju,

Issue fixed in version 1.13.0 as you can see from the snippet below:

https://github.com/STMicroelectronics/STM32CubeWB/blob/fd3d1c1ca7052bb526b9735b51a9109102c61bac/Utilities/sequencer/stm32_seq.c#L217-L223

Thank you again for having reported this point.

With regards,