This is a list of all comments for DG-DEN-16446-1.

Review Summary: No summary


----------------------------------------
File: firmware/App/Modes/ModeFill.c
Revision Comment by Sean Nash on 27 September 2023, 13:07 
https://devapps.diality.us/cru/DG-DEN-16446-1#c18974

  What's left?  When would this code ever become reachable?

  Reply by Dara Navaei on 28 September 2023, 08:33
  > This code is the theoretical code for treatment recovery. At
  > the moment this code is not reachable neither in the firmware
  > or VectorCAST.

    Reply by Sean Nash on 01 October 2023, 16:15
    > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: firmware/App/Controllers/Valves.c
Revision Comment by jtaylor on 27 September 2023, 08:29 
https://devapps.diality.us/cru/DG-DEN-16446-1#c18973

  The default case should probably be present, whether or not
  _VECTORCAST_ is defined.  I suggest bringing the ifdef in, to
  eliminate only the "name =" line (479).

  Reply by Dara Navaei on 28 September 2023, 08:30
  > This default is in an if statement to make sure we are less
  > than NUM_OF_VALVES so the default cannot be reached in
  > VectorCAST. But we keep the default in the code in case there
  > was a memory stomp or other anomalies.

    Reply by jtaylor on 29 September 2023, 11:13
    > RESOLVED in CODE WALKTHROUGH. (with nausea)

----------------------------------------
File: firmware/App/Modes/ModeHeatDisinfect.c
Revision Comment by jtaylor on 27 September 2023, 16:27 
https://devapps.diality.us/cru/DG-DEN-16446-1#c18976

  Our coding standard encourages always including default as the
  last clause, even if it is empty. (following misra 16.5).
  I would suggest using the ifdef within the body of the default
  clause, even with the statement duplicated, or adding an else
  clause to the ifndef with a second, empty default.

  Reply by Dara Navaei on 28 September 2023, 08:56
  > Done

    Reply by jtaylor on 29 September 2023, 11:08
    > But if _VECTORCAST_ is defined there is no default clause.
    > I'm suggesting:
    > ...
    > default:
    > #ifndef _VECTORCAST_
    > heatDisinfectUIState =
    > HEAT_DISINFECT_UI_STATE_HEAT_UP_WATER;
    > #endif
    > break;

      Reply by jtaylor on 29 September 2023, 11:15
      > The question was resolved in comments for earlier default
      > cases in this change. RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: firmware/App/Controllers/ROPump.c
Revision Comment by wbracken on 02 October 2023, 00:38 
https://devapps.diality.us/cru/DG-DEN-16446-1#c18986

  Should this have a set of parenthesis around the condition
  similar to line 437?

  Reply by Dara Navaei on 03 October 2023, 14:33
  > In the condition in line 437 we do a subtraction prior to
  > comparing so there are extra parentheses in line 437.

    Reply by wbracken on 06 October 2023, 10:18
    > Resolved in code review.

----------------------------------------
File: firmware/App/Services/SystemCommMessages.c
Revision Comment by wbracken on 08 October 2023, 17:54 
https://devapps.diality.us/cru/DG-DEN-16446-1#c19008

  sizeof should be first in if

  Reply by Dara Navaei on 09 October 2023, 09:20
  > Done

    Reply by wbracken on 09 October 2023, 11:06
    > RESOLVED IN CODE WALKTHROUGH


---

ID: DG-DEN-16446-1 https://devapps.diality.us/cru/DG-DEN-16446-1

Title: DG-DEN-16446_DG Dev Test Clean UP
Statement of Objectives:


State: Closed

Summary: 

Author: Dara Navaei
Moderator: Dara Navaei
Reviewers: (5 active, 2 completed*)
     Sean Nash   (*)
     wbracken   (*)
     jpaguio
     Vinayakam Mani
     Michael Garthwaite
     Darren Cox
     jtaylor