This is a list of all comments for DG-DEN-15973-2.

Review Summary: No summary


----------------------------------------
File: firmware/App/Controllers/Pressures.c
Revision Comment by Sean Nash on 14 September 2023, 09:29 
https://devapps.diality.us/cru/DG-DEN-15973-2#c18904

  These to values appear to be minimums.  Why are they being
  called maximums?

  Reply by Dara Navaei on 14 September 2023, 09:31
  > This is the minimum values that are needed to clear the
  > alarm.

    Reply by Sean Nash on 14 September 2023, 09:37
    > No, these are the minimum values to trigger the alarm.  But
    > they are named maximums.

      Reply by Dara Navaei on 14 September 2023, 09:44
      > Done

        Reply by Sean Nash on 14 September 2023, 09:53
        > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 14 September 2023, 09:29 
https://devapps.diality.us/cru/DG-DEN-15973-2#c18905

  These 2 values appear to be minimum recovery thresholds.

  Reply by Dara Navaei on 14 September 2023, 09:32
  > This is the minimum values that are needed to clear the
  > alarm.

    Reply by Sean Nash on 14 September 2023, 09:37
    > Yes, so name should reflect that.  These names suggest they
    > are minimum pressures below which we would alarm.

      Reply by Dara Navaei on 14 September 2023, 09:42
      > Done

        Reply by Sean Nash on 14 September 2023, 09:52
        > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: firmware/App/Modes/OperationModes.h
Revision Comment by Sean Nash on 14 September 2023, 09:35 
https://devapps.diality.us/cru/DG-DEN-15973-2#c18909

  Align comment.

  Reply by Dara Navaei on 14 September 2023, 09:38
  > Done

    Reply by Sean Nash on 14 September 2023, 09:50
    > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: firmware/App/Modes/ModeStandby.c
Revision Comment by Sean Nash on 14 September 2023, 09:35 
https://devapps.diality.us/cru/DG-DEN-15973-2#c18908

  Magic numbers?

  Reply by Dara Navaei on 14 September 2023, 09:37
  > Yes these are for software configuration disable only so I
  > did not make any #defines for them.

    Reply by Sean Nash on 14 September 2023, 09:38
    > You at least need to add a comment explaining what they
    > are.

      Reply by Sean Nash on 14 September 2023, 09:51
      > Dara, I mean inline comments for the magic numbers.
      > Explain what 40 and 32 mean and why you're using those
      > values.

        Reply by Dara Navaei on 14 September 2023, 10:28
        > Done

          Reply by Sean Nash on 14 September 2023, 10:58
          > RESOLVED in CODE WALKTHROUGH.


---

ID: DG-DEN-15973-2 https://devapps.diality.us/cru/DG-DEN-15973-2

Title: DG-DEN-15973_Alarm 48 UF Rate Tare Error Triggered During Treatment
Statement of Objectives:


State: Closed

Summary: 

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