This is a list of all comments for DG-DEN-14559-1. Review Summary: No summary ---------------------------------------- File: AlarmDefs.h Revision Comment by Sean Nash on 17 January 2023, 13:16 https://devapps.diality.us/cru/DG-DEN-14559-1#c16060 This alarm does not appear to be available. Correct description in last column (trigger condition). Reply by Dara Navaei on 18 January 2023, 15:54 > This alarm is still available. Reply by Sean Nash on 18 January 2023, 16:32 > Why does enum say otherwise (prime saline dialyzer time > out)? Reply by Dara Navaei on 19 January 2023, 08:14 > Done Reply by Sean Nash on 19 January 2023, 08:20 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModeChemicalDisinfect.c Revision Comment by jtaylor on 16 January 2023, 10:55 https://devapps.diality.us/cru/DG-DEN-14559-1#c16024 Alignment Reply by Dara Navaei on 18 January 2023, 15:56 > Done Reply by Sean Nash on 19 January 2023, 08:08 > RESOLVED in CODE WALKTHROUGH. Revision Comment by wbracken on 17 January 2023, 15:08 https://devapps.diality.us/cru/DG-DEN-14559-1#c16075 Seems like there are several variables missing. Reply by Dara Navaei on 18 January 2023, 15:43 > Done Reply by wbracken on 18 January 2023, 16:43 > RESOLVED IN CODE WALKTHROUGH Revision Comment by Sean Nash on 17 January 2023, 13:42 https://devapps.diality.us/cru/DG-DEN-14559-1#c16068 Prefer doing this kind of thing with a loop iterating through the enum in case we add/remove a reservoir. Reply by Dara Navaei on 18 January 2023, 15:49 > Done Reply by Sean Nash on 18 January 2023, 16:30 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 17 January 2023, 13:45 https://devapps.diality.us/cru/DG-DEN-14559-1#c16069 This looks strange. Why is TD2 temp usually 0.0, but read if using TPo? Reply by Dara Navaei on 18 January 2023, 15:46 > Done. Reply by Sean Nash on 18 January 2023, 16:29 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModeChemicalDisinfectFlush.c Revision Comment by wbracken on 16 January 2023, 12:11 https://devapps.diality.us/cru/DG-DEN-14559-1#c16025 Alignment Reply by Dara Navaei on 18 January 2023, 15:55 > DOne Reply by wbracken on 18 January 2023, 16:44 > RESOLVED IN CODE WALKTHROUGH Revision Comment by Sean Nash on 17 January 2023, 13:41 https://devapps.diality.us/cru/DG-DEN-14559-1#c16067 Does hasAlarmBeenTriggered need to be set here too? If not, need to clarify what that flag really means. Reply by Dara Navaei on 18 January 2023, 15:49 > No, at this point we are done with the variable. Reply by Sean Nash on 18 January 2023, 16:30 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModeFlush.c Revision Comment by wbracken on 18 January 2023, 16:35 https://devapps.diality.us/cru/DG-DEN-14559-1#c16126 Alignment Reply by Dara Navaei on 18 January 2023, 22:03 > Done Reply by Sean Nash on 19 January 2023, 08:07 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModeHeatDisinfect.c Revision Comment by wbracken on 18 January 2023, 16:36 https://devapps.diality.us/cru/DG-DEN-14559-1#c16127 Alignment Reply by Dara Navaei on 18 January 2023, 22:04 > Done Reply by Sean Nash on 19 January 2023, 08:07 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/AlarmMgmt.c Revision Comment by Sean Nash on 17 January 2023, 13:19 https://devapps.diality.us/cru/DG-DEN-14559-1#c16061 Name is too broad and comment is too specific. Should probably be a BOOL flag that indicates there is an active DG fault. Reply by Dara Navaei on 18 January 2023, 16:03 > Done Reply by Sean Nash on 18 January 2023, 16:32 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 17 January 2023, 13:23 https://devapps.diality.us/cru/DG-DEN-14559-1#c16063 Initialize to FALSE. Reply by Dara Navaei on 18 January 2023, 16:00 > Done Reply by Sean Nash on 18 January 2023, 16:31 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 17 January 2023, 13:24 https://devapps.diality.us/cru/DG-DEN-14559-1#c16064 Set to TRUE here. Reply by Dara Navaei on 18 January 2023, 15:59 > Done Reply by Sean Nash on 18 January 2023, 16:31 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 17 January 2023, 13:24 https://devapps.diality.us/cru/DG-DEN-14559-1#c16065 Variable should already be a BOOL. Reply by Dara Navaei on 18 January 2023, 15:58 > Done. Reply by Sean Nash on 18 January 2023, 16:31 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/AlarmMgmt.h Revision Comment by Sean Nash on 17 January 2023, 13:23 https://devapps.diality.us/cru/DG-DEN-14559-1#c16062 Rename to isDGFaultActive. Reply by Dara Navaei on 18 January 2023, 16:02 > Done. Reply by Sean Nash on 18 January 2023, 16:32 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: NVDataMgmt.c Revision Comment by Sean Nash on 17 January 2023, 13:07 https://devapps.diality.us/cru/DG-DEN-14559-1#c16059 Do we need to re-queue these reads? We've already done it at real POST. And if we do need to queue them, why does queue have to be empty? Shouldn't we always queue them if they're needed? Reply by Dara Navaei on 18 January 2023, 15:54 > No, this code has been removed. Reply by Sean Nash on 18 January 2023, 16:33 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModeInitPOST.c Revision Comment by Sean Nash on 17 January 2023, 13:27 https://devapps.diality.us/cru/DG-DEN-14559-1#c16066 Need to be careful with these. These functions are currently resetting the POST functions to start at beginning, but also need to reset ANY variables that the POST functions use/set (not override variables though). Also, since we are not re-initializing everything like a true reset does, we need to make sure that these modules are setup to restart or resume (whichever is more appropriate for the module) after POST completes if the module is in any way interrupted/deferred by POST or competing with POST for a resource/driver (e.g. RTC or NV-Data). Reply by Dara Navaei on 18 January 2023, 15:51 > That is true. I tested these functions and they reset only > the minimum (necessary) variables. Reply by Sean Nash on 18 January 2023, 16:31 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModeDrain.c Revision Comment by wbracken on 18 January 2023, 16:35 https://devapps.diality.us/cru/DG-DEN-14559-1#c16125 Alignment Reply by Dara Navaei on 18 January 2023, 22:02 > Done Reply by Sean Nash on 19 January 2023, 08:07 > RESOLVED in CODE WALKTHROUGH. --- ID: DG-DEN-14559-1 https://devapps.diality.us/cru/DG-DEN-14559-1 Title: DG-DEN-14559_DG HD Dev HD DG Dvt Update Part 8 Statement of Objectives: State: Closed Summary: Author: Dara Navaei Moderator: Dara Navaei Reviewers: (4 active, 2 completed*) Sean Nash (*) wbracken (*) Michael Garthwaite Darren Cox jtaylor Steve Jarpe