This is a list of all comments for HD-DEN-12224-16. Review Summary: No summary ---------------------------------------- File: firmware/App/Controllers/PresOccl.c Revision Comment by Sean Nash on 20 May 2022, 14:06 https://devapps.diality.us/cru/HD-DEN-12224-16#c12835 Remove extra blank line. And why is there any blank line here? Reply by Dara Navaei on 22 May 2022, 14:48 > Done. Reply by Sean Nash on 23 May 2022, 17:01 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ConsumableSelfTest.c Revision Comment by Sean Nash on 14 April 2022, 14:19 https://devapps.diality.us/cru/HD-DEN-12224-16#c12775 Why is this necessary. This is already called somewhere else. Reply by Sean Nash on 15 April 2022, 16:26 > Dara removed this line of code after lab test w/o this change > could not replicate the bug. Reply by Sean Nash on 23 May 2022, 17:15 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: AlarmDefs.h Revision Comment by Sean Nash on 18 May 2022, 15:12 https://devapps.diality.us/cru/HD-DEN-12224-16#c12817 For all DG faults, the NoRes should be TRUE, but the NoRin and NoEnd should be FALSE so user can perform rinseback and/or end treatment. Reply by Dara Navaei on 25 May 2022, 10:19 > This will be addressed in DEN-12931. Reply by Sean Nash on 25 May 2022, 10:25 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 18 May 2022, 15:29 https://devapps.diality.us/cru/HD-DEN-12224-16#c12820 NoBRcr should be TRUE and NoDRcr should be FALSE. Reply by Dara Navaei on 23 May 2022, 10:31 > Done. Reply by Sean Nash on 23 May 2022, 17:13 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 18 May 2022, 15:32 https://devapps.diality.us/cru/HD-DEN-12224-16#c12821 NoDRcr should be FALSE. Reply by Dara Navaei on 23 May 2022, 10:29 > Done. Reply by Sean Nash on 23 May 2022, 17:13 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: DGDefs.h Revision Comment by Sean Nash on 18 May 2022, 15:13 https://devapps.diality.us/cru/HD-DEN-12224-16#c12818 Do we need this state? Reply by Dara Navaei on 23 May 2022, 10:31 > We should keep this state until we are sure it is no longer > needed. Reply by Sean Nash on 23 May 2022, 17:14 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/SystemCommMessages.c Revision Comment by Michael Garthwaite on 23 May 2022, 10:54 https://devapps.diality.us/cru/HD-DEN-12224-16#c12918 couldnt we simplify this and compare it to a struct of the payload? (ex: UI_VERSIONS_T) Reply by Michael Garthwaite on 24 May 2022, 10:37 > RESOLVED IN CODE WALKTHROUGH. Revision Comment by Sean Nash on 20 May 2022, 15:41 https://devapps.diality.us/cru/HD-DEN-12224-16#c12839 Is this zero temporary? If so, add TODO. Reply by Dara Navaei on 22 May 2022, 14:43 > Done. Reply by Sean Nash on 23 May 2022, 16:57 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 20 May 2022, 15:41 https://devapps.diality.us/cru/HD-DEN-12224-16#c12840 Why isn't this command allowed for release version? Reply by Dara Navaei on 22 May 2022, 14:40 > Removed the release check. Reply by Sean Nash on 23 May 2022, 16:57 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 20 May 2022, 15:43 https://devapps.diality.us/cru/HD-DEN-12224-16#c12841 Remove blank line. Reply by Dara Navaei on 22 May 2022, 14:46 > Done. Reply by Sean Nash on 23 May 2022, 16:55 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 20 May 2022, 15:47 https://devapps.diality.us/cru/HD-DEN-12224-16#c12843 Remove blank line. Reply by Dara Navaei on 22 May 2022, 14:46 > Done. Reply by Sean Nash on 25 May 2022, 10:24 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 20 May 2022, 15:48 https://devapps.diality.us/cru/HD-DEN-12224-16#c12844 Remove blank line. Reply by Dara Navaei on 22 May 2022, 14:38 > Done Reply by Sean Nash on 24 May 2022, 17:04 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/SyringePump.c Revision Comment by Michael Garthwaite on 30 March 2022, 14:29 https://devapps.diality.us/cru/HD-DEN-12224-16#c12728 This variable is defined but not used. Reply by Sean Nash on 18 May 2022, 15:58 > It is used on lines 302, 1172, 1795, 1875. Reply by Michael Garthwaite on 23 May 2022, 16:41 > RESOLVED IN CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModePostTreat.c Revision Comment by Sean Nash on 20 May 2022, 15:01 https://devapps.diality.us/cru/HD-DEN-12224-16#c12837 Should we exit blood leak normal mode in here somewhere? Reply by Dara Navaei on 23 May 2022, 13:28 > Done. Reply by Sean Nash on 24 May 2022, 17:05 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/SelfTests.c Revision Comment by Michael Garthwaite on 30 March 2022, 14:40 https://devapps.diality.us/cru/HD-DEN-12224-16#c12730 did we want to keep these loose tolerances from the demo? Reply by Sean Nash on 18 May 2022, 16:03 > We will want to tighten these eventually. Reply by Dara Navaei on 23 May 2022, 10:33 > [~mgarthwaite] do you have the latest tolerances after your > testings? Reply by Michael Garthwaite on 23 May 2022, 11:02 > Not yet. Reply by Michael Garthwaite on 23 May 2022, 16:43 > RESOLVED IN CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/AirTrap.c Revision Comment by Sean Nash on 18 May 2022, 16:50 https://devapps.diality.us/cru/HD-DEN-12224-16#c12829 Remove extra blank line. Reply by Dara Navaei on 22 May 2022, 19:22 > Done. Reply by Sean Nash on 23 May 2022, 17:12 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/BloodLeak.c Revision Comment by Darren Cox on 12 May 2022, 14:48 https://devapps.diality.us/cru/HD-DEN-12224-16#c12810 One letter enums could be more descriptive with their names. Reply by Sean Nash on 18 May 2022, 16:35 > Agree. Dara, make these look something like > "C_CALIBRATION_CMD". Reply by Dara Navaei on 23 May 2022, 16:53 > Done. Reply by Darren Cox on 23 May 2022, 17:13 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 18 May 2022, 16:40 https://devapps.diality.us/cru/HD-DEN-12224-16#c12827 "reset" should be "rest" I think. Reply by Dara Navaei on 23 May 2022, 10:25 > Done. Reply by Sean Nash on 23 May 2022, 17:12 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 18 May 2022, 16:53 https://devapps.diality.us/cru/HD-DEN-12224-16#c12830 Should we set this flag only if we are in normal state? Reply by Dara Navaei on 22 May 2022, 19:20 > The other states ignore this flag and I have to set it here > since this is a request. Reply by Sean Nash on 24 May 2022, 17:06 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 18 May 2022, 16:45 https://devapps.diality.us/cru/HD-DEN-12224-16#c12828 Should this be set to FALSE? Reply by Dara Navaei on 22 May 2022, 19:22 > Done. Reply by Sean Nash on 23 May 2022, 17:12 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/DialOutFlow.c Revision Comment by Michael Garthwaite on 30 March 2022, 13:03 https://devapps.diality.us/cru/HD-DEN-12224-16#c12708 Do we need to keep this define? Reply by Sean Nash on 30 March 2022, 13:26 > I wanted to see how new #define worked. I think it's better, > so we can remove this now. Reply by Dara Navaei on 23 May 2022, 10:35 > Done. Reply by Michael Garthwaite on 23 May 2022, 16:40 > RESOLVED IN CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/BloodPrime.c Revision Comment by Sean Nash on 20 May 2022, 14:45 https://devapps.diality.us/cru/HD-DEN-12224-16#c12836 Ask Behrouz if UI is still looking for this. Remove if not. Reply by Dara Navaei on 25 May 2022, 10:17 > Done. Reply by Sean Nash on 25 May 2022, 10:25 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/Fans.c Revision Comment by Sean Nash on 20 May 2022, 14:04 https://devapps.diality.us/cru/HD-DEN-12224-16#c12833 Should be rpmAlarmStartTime Reply by Dara Navaei on 22 May 2022, 14:53 > Done Reply by Sean Nash on 23 May 2022, 17:02 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 20 May 2022, 13:57 https://devapps.diality.us/cru/HD-DEN-12224-16#c12831 I don't think this results in an elapsed time. It's just a different start time. The calcTimeSince below gives you the elapsed time. Reply by Dara Navaei on 23 May 2022, 10:25 > Done. Reply by Sean Nash on 23 May 2022, 17:07 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 20 May 2022, 14:00 https://devapps.diality.us/cru/HD-DEN-12224-16#c12832 Did we want this offset to revert to zero here or should Dialin have to do a reset? Reply by Dara Navaei on 22 May 2022, 14:53 > For safety I do it here. Reply by Sean Nash on 23 May 2022, 17:05 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 20 May 2022, 14:05 https://devapps.diality.us/cru/HD-DEN-12224-16#c12834 Seems like we're setting a start time offset here, not a time stamp. Reply by Dara Navaei on 22 May 2022, 14:51 > Done Reply by Sean Nash on 24 May 2022, 17:06 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/Voltages.h Revision Comment by Michael Garthwaite on 30 March 2022, 14:33 https://devapps.diality.us/cru/HD-DEN-12224-16#c12729 We typically put #define like this in the source file. Is there a reason why this one is in the header file specifically? Reply by Dara Navaei on 23 May 2022, 10:34 > Not if the #define is used by other modules. In this case, > this #define is used by ModeInitPOST.c Reply by Michael Garthwaite on 23 May 2022, 16:42 > RESOLVED IN CODE WALKTHROUGH. ---------------------------------------- File: NVDataMgmt.c Revision Comment by Darren Cox on 12 May 2022, 14:41 https://devapps.diality.us/cru/HD-DEN-12224-16#c12809 Why no SW Config in Release? Reply by Sean Nash on 18 May 2022, 15:44 > We want release s/w to be fully enabled with no special test > code. SW configs are structured such that a disabled config > is what is wanted for release s/w and so forcing all s/w > configs to be disabled in a release build ensures we don't > accidentally leave a config in the wrong state. Reply by Dara Navaei on 23 May 2022, 16:37 > We have to make sure the enable/disable feature in the > software releases is not available in the release. Reply by Darren Cox on 23 May 2022, 17:13 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Michael Garthwaite on 30 March 2022, 12:45 https://devapps.diality.us/cru/HD-DEN-12224-16#c12705 Do we need to keep this alarm? Reply by Dara Navaei on 23 May 2022, 13:17 > There will not be any alarms for the software configurations. Reply by Michael Garthwaite on 23 May 2022, 16:39 > RESOLVED IN CODE WALKTHROUGH. Revision Comment by Michael Garthwaite on 30 March 2022, 12:45 https://devapps.diality.us/cru/HD-DEN-12224-16#c12706 or this alarm? Reply by Dara Navaei on 23 May 2022, 13:17 > There will not be any alarms for the software configurations. Reply by Michael Garthwaite on 23 May 2022, 16:40 > RESOLVED IN CODE WALKTHROUGH. Revision Comment by Sean Nash on 23 May 2022, 15:10 https://devapps.diality.us/cru/HD-DEN-12224-16#c12967 Don't we still want to know if it's valid - and revert to defaults if not? Reply by Dara Navaei on 24 May 2022, 09:13 > This function is still available. Reply by Sean Nash on 24 May 2022, 17:03 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 23 May 2022, 15:09 https://devapps.diality.us/cru/HD-DEN-12224-16#c12966 Don't we still want to know if it's valid - and revert to defaults if not? Reply by Dara Navaei on 24 May 2022, 09:12 > This function is still available and reverts back to default. > It does not alarm though. Reply by Sean Nash on 24 May 2022, 17:03 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: RTC.c Revision Comment by Sean Nash on 23 May 2022, 15:11 https://devapps.diality.us/cru/HD-DEN-12224-16#c12968 Remove blank line. Reply by Dara Navaei on 24 May 2022, 09:13 > Done. Reply by Sean Nash on 24 May 2022, 17:02 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Michael Garthwaite on 23 May 2022, 10:14 https://devapps.diality.us/cru/HD-DEN-12224-16#c12909 Wont these alarms be with an unknown alarm id for hd? it is created but not assigned. Reply by Dara Navaei on 23 May 2022, 11:10 > Thanks Michael, fixed it. Reply by Michael Garthwaite on 23 May 2022, 16:44 > RESOLVED IN CODE WALKTHROUGH. Revision Comment by Sean Nash on 23 May 2022, 15:13 https://devapps.diality.us/cru/HD-DEN-12224-16#c12969 Why only DG? Reply by Dara Navaei on 24 May 2022, 09:25 > Done. Reply by Sean Nash on 24 May 2022, 17:02 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/TreatmentStop.c Revision Comment by Sean Nash on 20 May 2022, 15:02 https://devapps.diality.us/cru/HD-DEN-12224-16#c12838 I don't think we should exit normal mode in TreatmentStop sub-mode. Reply by Dara Navaei on 23 May 2022, 13:20 > Done. Reply by Sean Nash on 23 May 2022, 16:58 > RESOLVED in CODE WALKTHROUGH. --- ID: HD-DEN-12224-16 https://devapps.diality.us/cru/HD-DEN-12224-16 Title: HD-DEN-12224_DG HD Dev Switches Monitor Statement of Objectives: State: Closed Summary: Author: Dara Navaei Moderator: Dara Navaei Reviewers: (3 active, 0 completed*) Sean Nash Michael Garthwaite Darren Cox