This is a list of all comments for HD-DEN-16442-1.

Review Summary: No summary


----------------------------------------
File: firmware/App/Services/Reservoirs.c
Revision Comment by Sean Nash on 18 September 2023, 14:15 
https://devapps.diality.us/cru/HD-DEN-16442-1#c18929

  I think we have to remember last fill volume for each reservoir
  and refer to that.  
  With this approach, if user changes Qd, the percentage will be
  wrong.

  Reply by Dara Navaei on 18 September 2023, 21:00
  > Done

    Reply by Sean Nash on 19 September 2023, 11:25
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 19 September 2023, 11:36 
https://devapps.diality.us/cru/HD-DEN-16442-1#c18940

  I don't think we want to use recorded volume (would want
  inactive reservoir if we did).  I think we want to use your
  function here that calculates the target fill volume.

  Reply by Dara Navaei on 19 September 2023, 13:02
  > Done

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

Revision Comment by Sean Nash on 18 September 2023, 14:24 
https://devapps.diality.us/cru/HD-DEN-16442-1#c18932

  General issue - please consider appropriateness of using this
  calculation when calling this function.

  Reply by Dara Navaei on 18 September 2023, 21:00
  > Done

    Reply by Sean Nash on 19 September 2023, 11:26
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 19 September 2023, 11:30 
https://devapps.diality.us/cru/HD-DEN-16442-1#c18939

  Should we have a local var for active reservoir too?

  Reply by Dara Navaei on 19 September 2023, 11:49
  > Done

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

Revision Comment by Sean Nash on 19 September 2023, 11:29 
https://devapps.diality.us/cru/HD-DEN-16442-1#c18938

  Should this be active reservoir?

  Reply by Dara Navaei on 19 September 2023, 11:48
  > Done

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

----------------------------------------
File: firmware/App/Controllers/DGInterface.c
Revision Comment by Darren Cox on 19 September 2023, 16:00 
https://devapps.diality.us/cru/HD-DEN-16442-1#c18951

  Just these 2 alarms?  Should we check for any alarm that STOPS
  treatment?

  Reply by Dara Navaei on 19 September 2023, 16:10
  > Yes. The other alarms are handled in different drivers or
  > modes. The above alarms are here to make sure HD sends the
  > fill command after the bottles have been replaced.

    Reply by Darren Cox on 19 September 2023, 16:10
    > RESOLVED IN CODE WALKTHROUGH.

----------------------------------------
File: firmware/App/Modes/ModePostTreat.c
Revision Comment by Sean Nash on 20 September 2023, 13:03 
https://devapps.diality.us/cru/HD-DEN-16442-1#c18958

  I think this should be a constant array of floats with
  NUM_OF_ACID_CONC_TYPES elements and initialize here with 1.0,
  2.0, 3.0.

  Reply by Dara Navaei on 20 September 2023, 13:19
  > Done

    Reply by Sean Nash on 20 September 2023, 13:22
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 20 September 2023, 13:01 
https://devapps.diality.us/cru/HD-DEN-16442-1#c18956

  should be 2251_0

  Reply by Dara Navaei on 20 September 2023, 13:19
  > Done

    Reply by Sean Nash on 20 September 2023, 13:22
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 20 September 2023, 13:02 
https://devapps.diality.us/cru/HD-DEN-16442-1#c18957

  Should be 3251_9

  Reply by Dara Navaei on 20 September 2023, 13:19
  > Done

    Reply by Sean Nash on 20 September 2023, 13:25
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 20 September 2023, 13:05 
https://devapps.diality.us/cru/HD-DEN-16442-1#c18959

  You won't need a switch if you use an array per previous
  comment.

  Reply by Dara Navaei on 20 September 2023, 13:19
  > Done

    Reply by Sean Nash on 20 September 2023, 13:24
    > RESOLVED in CODE WALKTHROUGH.


---

ID: HD-DEN-16442-1 https://devapps.diality.us/cru/HD-DEN-16442-1

Title: HD-DEN-16442_Reservoir Fill Change For Clearance Performance Improvement
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