This is a list of all comments for DG-DEN-8030-1. Review Summary: No summary ---------------------------------------- File: firmware/App/Controllers/LoadCell.c Revision Comment by Sean Nash on 15 June 2021, 10:14 https://devapps.diality.us/cru/DG-DEN-8030-1#c10246 Should this be F64 as well? Reply by Dara Navaei on 16 June 2021, 21:36 > Done. Reply by Sean Nash on 17 June 2021, 10:35 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModeStandby.c Revision Comment by Sean Nash on 04 June 2021, 11:50 https://devapps.diality.us/cru/DG-DEN-8030-1#c10210 Why commented out? Reply by Dara Navaei on 13 June 2021, 16:19 > This was for testing. It is uncommented. Reply by Sean Nash on 16 June 2021, 10:40 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/SystemComm.c Revision Comment by Sean Nash on 04 June 2021, 12:26 https://devapps.diality.us/cru/DG-DEN-8030-1#c10211 Please keep this case in last position in switch statement. Reply by Dara Navaei on 13 June 2021, 16:18 > Done. Reply by Sean Nash on 16 June 2021, 10:39 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/SystemCommMessages.c Revision Comment by Sean Nash on 04 June 2021, 12:28 https://devapps.diality.us/cru/DG-DEN-8030-1#c10212 Not necessary - included in .h file. Reply by Dara Navaei on 13 June 2021, 16:17 > I removed it. Reply by Sean Nash on 16 June 2021, 10:38 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModeDrain.c Revision Comment by Sean Nash on 04 June 2021, 10:41 https://devapps.diality.us/cru/DG-DEN-8030-1#c10208 Remove comments or add TODO. Reply by Dara Navaei on 13 June 2021, 16:19 > Done. Reply by Sean Nash on 16 June 2021, 10:41 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 04 June 2021, 10:41 https://devapps.diality.us/cru/DG-DEN-8030-1#c10209 I think we can remove this test code now. Reply by Dara Navaei on 13 June 2021, 16:19 > Done. Reply by Sean Nash on 17 June 2021, 10:38 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModeChemicalDisinfect.c Revision Comment by Sean Nash on 04 June 2021, 12:37 https://devapps.diality.us/cru/DG-DEN-8030-1#c10215 Why are some values aligned at left and others at right? I think all values should align at left. Reply by Dara Navaei on 15 June 2021, 11:12 > Done. Reply by Sean Nash on 16 June 2021, 10:35 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 16 June 2021, 10:06 https://devapps.diality.us/cru/DG-DEN-8030-1#c10259 Remove TODO once the original has been brought back. Reply by Dara Navaei on 16 June 2021, 21:15 > Done. Reply by qnguyen on 17 June 2021, 10:57 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 16 June 2021, 10:17 https://devapps.diality.us/cru/DG-DEN-8030-1#c10262 Why is this check removed? Reply by Dara Navaei on 16 June 2021, 21:32 > I check in the inlet pressure only at the states that are > needed. There are times that I am not even pulling water from > the inlet line. Reply by qnguyen on 17 June 2021, 10:55 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 16 June 2021, 10:12 https://devapps.diality.us/cru/DG-DEN-8030-1#c10261 Should this value be higher? The user could be using wrong concentrate jug if it is too low? Reply by Dara Navaei on 16 June 2021, 21:23 > 2000 uS/cm is a safe value to determine if the acid is > inserted in. Due to sensor to sensor variations, we decided > to keep it a little lower than its conductivity value is. Reply by qnguyen on 17 June 2021, 10:55 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 04 June 2021, 12:39 https://devapps.diality.us/cru/DG-DEN-8030-1#c10216 I think we should at least say that the mode variables are reset to re-start the mode or something. Reply by Dara Navaei on 13 June 2021, 16:10 > Done Reply by Sean Nash on 16 June 2021, 10:35 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 04 June 2021, 12:42 https://devapps.diality.us/cru/DG-DEN-8030-1#c10217 Is this needed. Should we just let Standby mode set actuators as it wants on entry? Reply by Dara Navaei on 13 June 2021, 16:08 > I removed the function. Reply by Sean Nash on 16 June 2021, 10:34 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 04 June 2021, 14:06 https://devapps.diality.us/cru/DG-DEN-8030-1#c10218 I think we should add the new THd temperature sensor now. I know it's currently using TRo connection (and so we don't have a TRo), but we should handle that with build switch (THd reads from TRo channel and TRo can just be a copy of TDi for now if THD_USING_TRO_CONNECTOR build switch enabled, actual future code where both sensors exist on their own connectors if build switch is disabled). Reply by Dara Navaei on 13 June 2021, 17:22 > Done Reply by Sean Nash on 15 June 2021, 09:59 > Not quite what I had in mind. > I meant that, regardless of whether sharing a connector, > you would always "get" the new heat disinfect temperature. > Inside TempSensors, you would have the build switch. If > using TRO connector for new sensor, fill THd from TRo temp > sensor and fill TRo from TDi temp sensor (both TDi and TRo > share TDi sensor). If THd has its own connector (DVT > system?), then fill THd from THd sensor and TRo from TRo > sensor as you would normally expect. Reply by Dara Navaei on 15 June 2021, 13:29 > Done. Reply by Sean Nash on 16 June 2021, 10:34 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 16 June 2021, 10:02 https://devapps.diality.us/cru/DG-DEN-8030-1#c10258 Instead of retry logic, should we just double the time out? This retry is essentially extended the time one more without any action to correct the first time out. Reply by Dara Navaei on 21 June 2021, 10:01 > This a matter of trying the temperature sensors more than > once before we start the cycle. Making the wait time longer > will not necessarily help the sensors to read within the > range. Reply by qnguyen on 21 June 2021, 10:38 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 16 June 2021, 10:10 https://devapps.diality.us/cru/DG-DEN-8030-1#c10260 This should be a separate if. Otherwise, this else if is not reachable. Reply by Dara Navaei on 16 June 2021, 21:33 > Good catch, thanks. I changed it. Reply by qnguyen on 17 June 2021, 10:56 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 04 June 2021, 14:11 https://devapps.diality.us/cru/DG-DEN-8030-1#c10219 Blank line after declaration. Reply by Dara Navaei on 13 June 2021, 16:01 > Done. Reply by Sean Nash on 16 June 2021, 10:33 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 04 June 2021, 14:13 https://devapps.diality.us/cru/DG-DEN-8030-1#c10220 Change order of comparison. Reply by Dara Navaei on 13 June 2021, 16:00 > Done. Reply by Sean Nash on 16 June 2021, 10:33 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModeRecirculate.c Revision Comment by Sean Nash on 15 June 2021, 10:11 https://devapps.diality.us/cru/DG-DEN-8030-1#c10245 Remove dead code. Reply by Dara Navaei on 15 June 2021, 11:04 > Done. Reply by Sean Nash on 16 June 2021, 10:32 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 15 June 2021, 10:10 https://devapps.diality.us/cru/DG-DEN-8030-1#c10244 Yes, please remove. HD should set the target temp. I assume this was for dialysate heating tests. Reply by Dara Navaei on 15 June 2021, 11:05 > Done. Reply by Sean Nash on 16 June 2021, 10:33 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModeFill.c Revision Comment by qnguyen on 16 June 2021, 10:26 https://devapps.diality.us/cru/DG-DEN-8030-1#c10263 Do we need this include? Reply by Dara Navaei on 16 June 2021, 21:28 > It is no longer needed. Reply by qnguyen on 17 June 2021, 10:53 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModeHeatDisinfect.c Revision Comment by qnguyen on 16 June 2021, 10:29 https://devapps.diality.us/cru/DG-DEN-8030-1#c10264 Target speed has to set to get direction going before turning the pumps on. Reply by Dara Navaei on 17 June 2021, 09:56 > CONC_PUMPS_REVERSE_SPEED_ML_PER_MIN = -30.0 to indicate that > the direction is in reverse. Reply by qnguyen on 17 June 2021, 10:59 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/.launches/DG.launch Revision Comment by qnguyen on 16 June 2021, 09:29 https://devapps.diality.us/cru/DG-DEN-8030-1#c10255 do we need to resolve this conflict? Reply by Dara Navaei on 16 June 2021, 09:32 > Done. Reply by qnguyen on 16 June 2021, 10:43 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/source/notification.c Revision Comment by Sean Nash on 04 June 2021, 12:31 https://devapps.diality.us/cru/DG-DEN-8030-1#c10213 I'm assuming HET was enabled to get access to a GPIO pin? If so, do we need these notifications? Is there a place in HalCOGen to disable HET interrupts? Reply by Dara Navaei on 16 June 2021, 20:31 > These functions are automatically generated once a HET driver > is enabled in HALCoGen. Reply by Sean Nash on 17 June 2021, 10:36 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/Heaters.c Revision Comment by qnguyen on 16 June 2021, 09:45 https://devapps.diality.us/cru/DG-DEN-8030-1#c10257 Should this flag be deleted since we no longer have any V2 DG? Reply by Dara Navaei on 16 June 2021, 21:07 > We should keep this for now in case we wanted to lower the > heaters duty cycle to previous limits. Reply by qnguyen on 17 June 2021, 10:57 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/DrainPump.c Revision Comment by Sean Nash on 15 June 2021, 10:15 https://devapps.diality.us/cru/DG-DEN-8030-1#c10247 Please remove dead code. Reply by Dara Navaei on 15 June 2021, 10:19 > Done. Reply by Sean Nash on 16 June 2021, 10:31 > RESOLVED in CODE WALKTHROUGH. --- ID: DG-DEN-8030-1 https://devapps.diality.us/cru/DG-DEN-8030-1 Title: DG-DEN-8030_DG-HD DEV Mode Chemical Disinfect Statement of Objectives: State: Closed Summary: Author: Dara Navaei Moderator: Dara Navaei Reviewers: (3 active, 0 completed*) qnguyen Sean Nash pmontazemi