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

Review Summary: No summary


----------------------------------------
File: firmware/App/Controllers/DrainPump.c
Revision Comment by Sean Nash on 29 December 2020, 08:41 
https://devapps.diality.us/cru/DG-DEN-6080-1#c6865

  Assume this is 10% (not 0.1%), so not technically a percent.

  Reply by Dara Navaei on 29 December 2020, 10:17
  > Done

    Reply by Sean Nash on 29 December 2020, 10:32
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by pmontazemi on 29 December 2020, 10:15 
https://devapps.diality.us/cru/DG-DEN-6080-1#c6876

  Start comment with capital letter for consistency with the
  other lines.

  Reply by Dara Navaei on 29 December 2020, 10:22
  > Done

    Reply by pmontazemi on 29 December 2020, 10:38
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by pmontazemi on 29 December 2020, 10:15 
https://devapps.diality.us/cru/DG-DEN-6080-1#c6877

  Start comment with capital letter for consistency with the
  other lines.

  Reply by Dara Navaei on 29 December 2020, 10:22
  > Done

    Reply by pmontazemi on 29 December 2020, 10:38
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by pmontazemi on 30 December 2020, 11:55 
https://devapps.diality.us/cru/DG-DEN-6080-1#c6940

  Both inputs and outputs are currentDrainPumpRPM?

  Reply by Dara Navaei on 30 December 2020, 12:27
  > Yes, this global variable is an input which is updated with
  > the newest RPM that is converted from an ADC value from FPGA.

    Reply by pmontazemi on 30 December 2020, 14:39
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by qnguyen on 28 December 2020, 20:27 
https://devapps.diality.us/cru/DG-DEN-6080-1#c6863

  Suggest using a local variable to store target drain pump RPM
  value to avoid calling the same function twice.

  Reply by Dara Navaei on 29 December 2020, 10:21
  > Done

    Reply by qnguyen on 29 December 2020, 10:36
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by qnguyen on 28 December 2020, 20:25 
https://devapps.diality.us/cru/DG-DEN-6080-1#c6862

  Use parentheses to separate conditions.

  Reply by Dara Navaei on 29 December 2020, 10:20
  > Done

    Reply by qnguyen on 29 December 2020, 10:36
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 29 December 2020, 09:10 
https://devapps.diality.us/cru/DG-DEN-6080-1#c6869

  Any time we're pulling the safety line, there should be a fault
  triggered as well - otherwise s/w will be expecting things to
  continue operating normally and, of course, with safety line
  pulled they won't - causing a potential slew of alarms that
  won't make sense.

  Reply by Dara Navaei on 29 December 2020, 13:19
  > I added a persistent alarm for it.

    Reply by Sean Nash on 30 December 2020, 14:04
    > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: firmware/App/Controllers/ROPump.c
Revision Comment by Sean Nash on 29 December 2020, 08:38 
https://devapps.diality.us/cru/DG-DEN-6080-1#c6864

  I don't see alarm ID split in this review (AlarmDefs.h).  Add
  common repositories to this review.

  Reply by Dara Navaei on 29 December 2020, 09:09
  > Done

    Reply by Sean Nash on 29 December 2020, 10:32
    > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: AlarmDefs.h
Revision Comment by qnguyen on 29 December 2020, 09:18 
https://devapps.diality.us/cru/DG-DEN-6080-1#c6871

  Add new alarms to the below alarm table.

  Reply by Dara Navaei on 29 December 2020, 10:16
  > Done

    Reply by qnguyen on 29 December 2020, 10:36
    > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: DGDefs.h
Revision Comment by Sean Nash on 29 December 2020, 09:17 
https://devapps.diality.us/cru/DG-DEN-6080-1#c6870

  Need comments for Doxygen.

  Reply by Dara Navaei on 29 December 2020, 10:13
  > Done

    Reply by Sean Nash on 29 December 2020, 10:32
    > RESOLVED in CODE WALKTHROUGH.


---

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

Title: DG-DEN-6080_Revisit DG Drain Pump Control Scheme
Statement of Objectives:


State: Closed

Summary: 

Author: Dara Navaei
Reviewers: (0 active, 3 completed*)
     qnguyen   (*)
     Sean Nash   (*)
     pmontazemi   (*)