This is a list of all comments for HD-DEN-11750-2. Review Summary: No summary ---------------------------------------- File: firmware/App/Controllers/BloodFlow.c Revision Comment by Sean Nash on 02 March 2022, 11:08 https://devapps.diality.us/cru/HD-DEN-11750-2#c12208 Fix indenting here. Reply by Dara Navaei on 02 March 2022, 13:46 > Done. Reply by Sean Nash on 03 March 2022, 14:01 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/Fans.c Revision Comment by Sean Nash on 02 March 2022, 11:30 https://devapps.diality.us/cru/HD-DEN-11750-2#c12213 I think we should use <> for library includes. And they should go on top. Reply by Dara Navaei on 02 March 2022, 13:54 > Done. Reply by Sean Nash on 03 March 2022, 14:03 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Michael Garthwaite on 24 February 2022, 13:41 https://devapps.diality.us/cru/HD-DEN-11750-2#c12061 based on the struct, fanStatus.dutycycle does not need to be initialized NUM_OF_FANS_NAMES times. Reply by Dara Navaei on 26 February 2022, 12:30 > Good point, I updated it. Reply by Michael Garthwaite on 03 March 2022, 13:24 > RESOVLED IN CODE WALKTHROUGH. Reply by Dara Navaei on 19 October 2023, 09:24 > RESOLVED in CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Controllers/Fans.h Revision Comment by Sean Nash on 02 March 2022, 11:28 https://devapps.diality.us/cru/HD-DEN-11750-2#c12212 Remove blank line. And what is new remove field for? Reply by Dara Navaei on 02 March 2022, 13:52 > Done. Reply by Sean Nash on 03 March 2022, 13:59 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/SystemCommMessages.c Revision Comment by Sean Nash on 02 March 2022, 17:20 https://devapps.diality.us/cru/HD-DEN-11750-2#c12253 These appear to be Dialin message functions. They should be down in the Dialin section below the TEST SUPPORT FUNCTIONS banner. Reply by Dara Navaei on 03 March 2022, 12:50 > Done. Reply by Sean Nash on 03 March 2022, 13:53 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 02 March 2022, 17:10 https://devapps.diality.us/cru/HD-DEN-11750-2#c12252 Check payload length is at least 12. Reply by Dara Navaei on 03 March 2022, 12:48 > Done. Reply by Sean Nash on 03 March 2022, 13:53 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/AirTrap.c Revision Comment by Sean Nash on 02 March 2022, 11:19 https://devapps.diality.us/cru/HD-DEN-11750-2#c12211 Have you tried a release build to see if we have this right? Reply by Dara Navaei on 02 March 2022, 13:51 > Yes, I tried a release build and it compiled. Reply by Sean Nash on 02 March 2022, 15:56 > It doesn't appear to build for me. Reply by Dara Navaei on 02 March 2022, 16:05 > Yes, there might be other release issues. We have not > built release in a long time but we can resolve them once > we decided to have a release build. Reply by Sean Nash on 03 March 2022, 14:00 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/DGInterface.c Revision Comment by Michael Garthwaite on 24 February 2022, 13:34 https://devapps.diality.us/cru/HD-DEN-11750-2#c12060 Do these variables need to be volatile? Reply by Dara Navaei on 26 February 2022, 12:30 > [~snash] please reply. Reply by Sean Nash on 02 March 2022, 11:14 > They do not. Remove volatile from these declarations. Reply by Michael Garthwaite on 03 March 2022, 13:33 > RESOVLED IN CODE WALKTHROUGH. Reply by Dara Navaei on 19 October 2023, 09:40 > RESOLVED in CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Controllers/FluidLeak.c Revision Comment by Michael Garthwaite on 24 February 2022, 13:45 https://devapps.diality.us/cru/HD-DEN-11750-2#c12062 Is this supposed to be assigned to zero? it is unlike the other variables that have been changed Reply by Dara Navaei on 26 February 2022, 12:28 > I uninitialized it. Reply by Michael Garthwaite on 03 March 2022, 13:24 > RESOVLED IN CODE WALKTHROUGH. Reply by Dara Navaei on 19 October 2023, 09:24 > RESOLVED in CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Controllers/PresOccl.c Revision Comment by Michael Garthwaite on 24 February 2022, 13:47 https://devapps.diality.us/cru/HD-DEN-11750-2#c12063 Did we ever get clarification on these threshold values? Reply by Sean Nash on 02 March 2022, 15:54 > I reviewed these - fixed any differences due to merge. There > is one more threshold change pending that I'm aware of. I > will make in next branch. Reply by Michael Garthwaite on 03 March 2022, 13:31 > RESOVLED IN CODE WALKTHROUGH. Reply by Dara Navaei on 19 October 2023, 09:24 > RESOLVED in CODE WALKTHROUGH Revision Comment by Sean Nash on 02 March 2022, 11:39 https://devapps.diality.us/cru/HD-DEN-11750-2#c12215 Magic #. Reply by Dara Navaei on 02 March 2022, 14:05 > Done. Reply by Sean Nash on 03 March 2022, 13:57 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 02 March 2022, 11:38 https://devapps.diality.us/cru/HD-DEN-11750-2#c12214 Seems we are not consistent about using #ifndef _RELEASE_. We have it here, but not in the if above on line 400. Reply by Dara Navaei on 02 March 2022, 13:59 > Yes, I have a TODO to finish them off in DEN-12224. Reply by Sean Nash on 03 March 2022, 13:57 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/SyringePump.c Revision Comment by Sean Nash on 02 March 2022, 16:24 https://devapps.diality.us/cru/HD-DEN-11750-2#c12238 Add one more blank line here. Reply by Dara Navaei on 02 March 2022, 16:31 > It has been removed. Reply by Sean Nash on 02 March 2022, 16:36 > There should be 2 blank lines above and below the TEST > SUPPORT FUNCTIONS banner below. Reply by Sean Nash on 03 March 2022, 14:02 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/Temperatures.c Revision Comment by Sean Nash on 02 March 2022, 16:26 https://devapps.diality.us/cru/HD-DEN-11750-2#c12240 Add TODO to remove. Reply by Dara Navaei on 02 March 2022, 16:34 > Done. Reply by Sean Nash on 03 March 2022, 13:56 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 02 March 2022, 16:27 https://devapps.diality.us/cru/HD-DEN-11750-2#c12241 Add TODO to remove later. Reply by Dara Navaei on 02 March 2022, 16:34 > Done. Reply by Sean Nash on 03 March 2022, 13:55 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/BloodPrime.c Revision Comment by Michael Garthwaite on 24 February 2022, 14:04 https://devapps.diality.us/cru/HD-DEN-11750-2#c12065 Should 300 be a magic number? Reply by Dara Navaei on 26 February 2022, 12:27 > [~snash] please reply. Reply by Sean Nash on 02 March 2022, 16:34 > Can leave uninitialized here. It is initialized in init > function below. Reply by Michael Garthwaite on 03 March 2022, 13:29 > RESOVLED IN CODE WALKTHROUGH. Reply by Dara Navaei on 19 October 2023, 09:25 > RESOLVED in CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Modes/ModeInitPOST.c Revision Comment by Sean Nash on 02 March 2022, 16:43 https://devapps.diality.us/cru/HD-DEN-11750-2#c12248 Should this be in an else below? Reply by Dara Navaei on 02 March 2022, 20:45 > No, this should be removed. If this is in an else, in a > release build it is not going to compile sine the if will be > ignored. Reply by Sean Nash on 03 March 2022, 14:02 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/Rinseback.c Revision Comment by Michael Garthwaite on 24 February 2022, 14:16 https://devapps.diality.us/cru/HD-DEN-11750-2#c12066 Should 300 be a magic number? Reply by Dara Navaei on 26 February 2022, 12:27 > [~snash] please respond,. Reply by Sean Nash on 02 March 2022, 17:02 > This can be uninitialized. It is initialized in the init > function below. Reply by Michael Garthwaite on 03 March 2022, 13:29 > RESOVLED IN CODE WALKTHROUGH. Reply by Dara Navaei on 19 October 2023, 09:40 > RESOLVED in CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Services/SystemCommMessages.h Revision Comment by Sean Nash on 02 March 2022, 17:09 https://devapps.diality.us/cru/HD-DEN-11750-2#c12251 Remove extra blank line. Reply by Dara Navaei on 02 March 2022, 20:49 > Done. Reply by Sean Nash on 03 March 2022, 13:53 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/AlarmMgmt.c Revision Comment by Michael Garthwaite on 24 February 2022, 14:18 https://devapps.diality.us/cru/HD-DEN-11750-2#c12067 Do we want to keep this commented out defs? Reply by Dara Navaei on 26 February 2022, 12:26 > This code has been removed already. Reply by Michael Garthwaite on 03 March 2022, 13:26 > RESOVLED IN CODE WALKTHROUGH. Reply by Dara Navaei on 19 October 2023, 09:27 > RESOLVED in CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Services/FPGA.c Revision Comment by Michael Garthwaite on 24 February 2022, 14:20 https://devapps.diality.us/cru/HD-DEN-11750-2#c12068 Venous pressure does not have an error count. Is it needed? Reply by Michael Garthwaite on 03 March 2022, 13:28 > RESOVLED IN CODE WALKTHROUGH. Reply by Dara Navaei on 19 October 2023, 09:27 > RESOLVED in CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Tasks/TaskPriority.c Revision Comment by Michael Garthwaite on 24 February 2022, 14:29 https://devapps.diality.us/cru/HD-DEN-11750-2#c12070 Shouldn't this include be in TaskGeneral.c? Reply by Dara Navaei on 26 February 2022, 12:23 > It must be removed. Reply by Michael Garthwaite on 03 March 2022, 13:26 > RESOVLED IN CODE WALKTHROUGH. Reply by Dara Navaei on 19 October 2023, 09:27 > RESOLVED in CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Services/Reservoirs.c Revision Comment by Michael Garthwaite on 24 February 2022, 14:24 https://devapps.diality.us/cru/HD-DEN-11750-2#c12069 Is the commented test code still needed? Reply by Dara Navaei on 26 February 2022, 12:23 > Yes. We might go back to it. Reply by Michael Garthwaite on 03 March 2022, 13:27 > RESOVLED IN CODE WALKTHROUGH. Reply by Dara Navaei on 19 October 2023, 09:28 > RESOLVED in CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Drivers/Battery.c Revision Comment by Michael Garthwaite on 24 February 2022, 14:03 https://devapps.diality.us/cru/HD-DEN-11750-2#c12064 There are no changes in this file and its not a new file. Reply by Dara Navaei on 26 February 2022, 12:28 > That is right. Reply by Michael Garthwaite on 03 March 2022, 13:25 > RESOVLED IN CODE WALKTHROUGH. Reply by Dara Navaei on 19 October 2023, 09:28 > RESOLVED in CODE WALKTHROUGH --- ID: HD-DEN-11750-2 https://devapps.diality.us/cru/HD-DEN-11750-2 Title: HD-DEN-11750_DG Dev Dialysate Temperature Control Tune Up Statement of Objectives: State: Closed Summary: Author: Dara Navaei Moderator: Dara Navaei Reviewers: (2 active, 2 completed*) Sean Nash (*) Michael Garthwaite (*) hnguyen Behrouz NematiPour