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

Review Summary: No summary


----------------------------------------
File: firmware/App/Services/SystemCommMessages.c
Revision Comment by Sean Nash on 04 January 2022, 09:23 
https://devapps.diality.us/cru/DG-DEN-11114-1#c11653

  Add blank line between declarations and code.

  Reply by Dara Navaei on 04 January 2022, 22:21
  > Done.

    Reply by Sean Nash on 05 January 2022, 10:18
    > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: firmware/App/Controllers/ConcentratePumps.c
Revision Comment by Sean Nash on 04 January 2022, 08:38 
https://devapps.diality.us/cru/DG-DEN-11114-1#c11630

  I'm not understanding why + 1 is needed for these.

  Reply by Dara Navaei on 04 January 2022, 22:54
  > Done.

    Reply by Sean Nash on 05 January 2022, 10:29
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 04 January 2022, 08:36 
https://devapps.diality.us/cru/DG-DEN-11114-1#c11629

  Let's just solve this now instead of commenting it out.

  Reply by Dara Navaei on 04 January 2022, 22:56
  > I uncommented the code. I will continue monitoring this error
  > to see if it occurs again.

    Reply by Sean Nash on 05 January 2022, 10:29
    > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: firmware/App/Controllers/ROPump.c
Revision Comment by Sean Nash on 04 January 2022, 08:55 
https://devapps.diality.us/cru/DG-DEN-11114-1#c11640

  Blank line between declaration and code.  
  Fix indentation.
  Fix magic number 1000.0.

  Reply by Dara Navaei on 04 January 2022, 22:29
  > Done.

    Reply by Sean Nash on 05 January 2022, 10:23
    > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: firmware/App/Modes/ModeFill.c
Revision Comment by Sean Nash on 04 January 2022, 09:06 
https://devapps.diality.us/cru/DG-DEN-11114-1#c11642

  If no test support functions, remove this banner.

  Reply by Dara Navaei on 04 January 2022, 22:27
  > Done.

    Reply by Sean Nash on 05 January 2022, 10:22
    > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: firmware/App/Controllers/Fans.c
Revision Comment by hnguyen on 04 January 2022, 10:51 
https://devapps.diality.us/cru/DG-DEN-11114-1#c11695

  Insert a space between the last '0' and '}' in OVERRIDE_32_T
  type.

  Reply by Dara Navaei on 04 January 2022, 22:19
  > Done.

    Reply by hnguyen on 05 January 2022, 10:30
    > RESOLVED IN CODE WALKTHROUGH

Revision Comment by Sean Nash on 04 January 2022, 08:42 
https://devapps.diality.us/cru/DG-DEN-11114-1#c11631

  Should use OVERRIDE_RESET (which is 0) here.

  Reply by Dara Navaei on 04 January 2022, 22:51
  > Done.

    Reply by Sean Nash on 05 January 2022, 10:28
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by hnguyen on 04 January 2022, 11:11 
https://devapps.diality.us/cru/DG-DEN-11114-1#c11696

  Remove blank line here.

  Reply by Dara Navaei on 04 January 2022, 22:19
  > Done.

    Reply by hnguyen on 05 January 2022, 10:30
    > RESOLVED IN CODE WALKTHROUGH

----------------------------------------
File: firmware/App/Services/Reservoirs.c
Revision Comment by Sean Nash on 04 January 2022, 09:12 
https://devapps.diality.us/cru/DG-DEN-11114-1#c11647

  Should these come from calibration record or ok to hard code?

  Reply by Dara Navaei on 04 January 2022, 22:25
  > I will implement this in DEN-11750.

    Reply by Sean Nash on 05 January 2022, 10:21
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 04 January 2022, 09:15 
https://devapps.diality.us/cru/DG-DEN-11114-1#c11648

  Remove extra space after F32.

  Reply by Dara Navaei on 04 January 2022, 22:25
  > Done.

    Reply by Sean Nash on 05 January 2022, 10:20
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 04 January 2022, 09:16 
https://devapps.diality.us/cru/DG-DEN-11114-1#c11650

  Add blank line between declarations and code.

  Reply by Dara Navaei on 04 January 2022, 22:24
  > Done.

    Reply by Sean Nash on 05 January 2022, 10:19
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 04 January 2022, 09:16 
https://devapps.diality.us/cru/DG-DEN-11114-1#c11649

  I prefer to declare variables at top of scope (after '{'
  above).

  Reply by Dara Navaei on 04 January 2022, 22:23
  > Done.

    Reply by Sean Nash on 05 January 2022, 10:20
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 04 January 2022, 09:17 
https://devapps.diality.us/cru/DG-DEN-11114-1#c11651

  Remove extra blank line.

  Reply by Dara Navaei on 04 January 2022, 22:22
  > Done.

    Reply by Sean Nash on 05 January 2022, 10:19
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 04 January 2022, 09:17 
https://devapps.diality.us/cru/DG-DEN-11114-1#c11652

  Add alarm.

  Reply by Dara Navaei on 05 January 2022, 10:19
  > I will add this in DEN-11750.

    Reply by Sean Nash on 05 January 2022, 10:19
    > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: firmware/App/Services/Reservoirs.h
Revision Comment by Sean Nash on 04 January 2022, 09:07 
https://devapps.diality.us/cru/DG-DEN-11114-1#c11643

  Let's decide this now and remove TODO.

  Reply by Dara Navaei on 05 January 2022, 09:44
  > Done.

    Reply by Sean Nash on 05 January 2022, 10:22
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 04 January 2022, 09:08 
https://devapps.diality.us/cru/DG-DEN-11114-1#c11644

  Comment doesn't match code (1 vs 2 degrees).  
  What is the TODO here?  Is this temporary code?

  Reply by Dara Navaei on 05 January 2022, 09:38
  > Removed the comment.

    Reply by Sean Nash on 05 January 2022, 10:22
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 04 January 2022, 09:09 
https://devapps.diality.us/cru/DG-DEN-11114-1#c11645

  Add doxygen comments.

  Reply by Dara Navaei on 05 January 2022, 09:37
  > Done.

    Reply by Sean Nash on 05 January 2022, 10:21
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 04 January 2022, 09:10 
https://devapps.diality.us/cru/DG-DEN-11114-1#c11646

  This comment seems redundant.

  Reply by Dara Navaei on 04 January 2022, 22:27
  > I removed the comment.

    Reply by Sean Nash on 05 January 2022, 10:21
    > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: firmware/App/Controllers/Heaters.c
Revision Comment by Sean Nash on 04 January 2022, 08:47 
https://devapps.diality.us/cru/DG-DEN-11114-1#c11632

  ???

  Reply by Dara Navaei on 04 January 2022, 22:49
  > This was commented out for the performance and to make sure
  > the trimmer heater is not running.

    Reply by Sean Nash on 05 January 2022, 10:28
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 04 January 2022, 08:48 
https://devapps.diality.us/cru/DG-DEN-11114-1#c11633

  Can we remove this now?

  Reply by Dara Navaei on 04 January 2022, 22:48
  > I will remove this in DEN-11750. This is still used for data
  > collection.

    Reply by Sean Nash on 05 January 2022, 10:27
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 04 January 2022, 08:49 
https://devapps.diality.us/cru/DG-DEN-11114-1#c11634

  I think we need to restore this code.

  Reply by Dara Navaei on 04 January 2022, 22:47
  > I will bring this back in DEN-11750.

    Reply by Sean Nash on 05 January 2022, 10:26
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 04 January 2022, 08:51 
https://devapps.diality.us/cru/DG-DEN-11114-1#c11636

  Yes, I think we do need to check efficiency.

  Reply by Dara Navaei on 04 January 2022, 22:33
  > I will bring efficiency back in DEN-11750.

    Reply by Sean Nash on 05 January 2022, 10:25
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 04 January 2022, 08:50 
https://devapps.diality.us/cru/DG-DEN-11114-1#c11635

  I think we have macros in common.h to do this kind of thing.

  Reply by Dara Navaei on 04 January 2022, 22:46
  > Done.

    Reply by Sean Nash on 05 January 2022, 10:26
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 04 January 2022, 08:51 
https://devapps.diality.us/cru/DG-DEN-11114-1#c11637

  Remove extra blank line.

  Reply by Dara Navaei on 04 January 2022, 22:32
  > Done.

    Reply by Sean Nash on 05 January 2022, 10:25
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 04 January 2022, 08:52 
https://devapps.diality.us/cru/DG-DEN-11114-1#c11638

  Why is this code commented out?

  Reply by Dara Navaei on 05 January 2022, 09:27
  > The heaters sensors are not used to monitor the internal
  > temperature of the heaters. I added a build switch.

    Reply by Sean Nash on 05 January 2022, 10:25
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 04 January 2022, 08:52 
https://devapps.diality.us/cru/DG-DEN-11114-1#c11639

  Why is this code commented out?

  Reply by Dara Navaei on 05 January 2022, 09:15
  > The heaters sensors are not used to monitor the internal
  > temperature of the heaters. I added a build switch.

    Reply by Sean Nash on 05 January 2022, 10:24
    > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: firmware/App/Controllers/TemperatureSensors.h
Revision Comment by Sean Nash on 04 January 2022, 08:57 
https://devapps.diality.us/cru/DG-DEN-11114-1#c11641

  Can we remove these now?  Update message list and UI unhandled
  message conf file.

  Reply by Dara Navaei on 04 January 2022, 22:29
  > I would like to keep these for now until the heaters sensors
  > are completely out.

    Reply by Sean Nash on 05 January 2022, 10:23
    > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: firmware/App/DGCommon.h
Revision Comment by Sean Nash on 04 January 2022, 09:25 
https://devapps.diality.us/cru/DG-DEN-11114-1#c11654

  Is this build switch still used?

  Reply by Dara Navaei on 04 January 2022, 22:20
  > I kept in case we wanted to use it.

    Reply by Sean Nash on 05 January 2022, 10:18
    > RESOLVED in CODE WALKTHROUGH.


---

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

Title: DG-DEN-11114_Dialysate Temperature Control (2 of 2)
Statement of Objectives:


State: Closed

Summary: 

Author: Dara Navaei
Moderator: Dara Navaei
Reviewers: (1 active, 2 completed*)
     Sean Nash   (*)
     hnguyen   (*)
     Behrouz NematiPour