This is a list of all comments for HD-DEN-7605-2. Review Summary: No summary ---------------------------------------- File: firmware/App/Controllers/Voltages.c Revision Comment by qnguyen on 03 May 2021, 09:00 https://devapps.diality.us/cru/HD-DEN-7605-2#c9684 Persistence spell error. Reply by Sean Nash on 03 May 2021, 10:21 > Fixed. Reply by qnguyen on 04 May 2021, 10:37 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/SyringePump.c Revision Comment by pmontazemi on 03 May 2021, 10:01 https://devapps.diality.us/cru/HD-DEN-7605-2#c9687 Change comment from 11 ml to 13 ml if 13 ml is the current placeholder. Reply by Sean Nash on 03 May 2021, 10:17 > 13 was temporary change for full travel test with Mike. I > put it back to 11 for now. Reply by pmontazemi on 04 May 2021, 10:32 > RESOLVED in CODE WALKTHROUGH. Revision Comment by pmontazemi on 03 May 2021, 10:01 https://devapps.diality.us/cru/HD-DEN-7605-2#c9688 No more self-test (and associated self-test state machine) for heparin pump? Reply by Sean Nash on 03 May 2021, 10:15 > I don't have anything I can do in POST so I removed. There > are some checks being done at all times and many checks > happening in certain states. Reply by pmontazemi on 04 May 2021, 10:32 > RESOLVED in CODE WALKTHROUGH. Revision Comment by pmontazemi on 03 May 2021, 10:09 https://devapps.diality.us/cru/HD-DEN-7605-2#c9689 == FALSE instead of != TRUE? Reply by Sean Nash on 03 May 2021, 10:14 > I think it is safer as is. TRUE = 1. If any other value, > will be viewed as FALSE and check fails. Reply by pmontazemi on 04 May 2021, 10:32 > RESOLVED in CODE WALKTHROUGH. Revision Comment by pmontazemi on 03 May 2021, 10:10 https://devapps.diality.us/cru/HD-DEN-7605-2#c9690 Parentheses unnecessary. Reply by Sean Nash on 03 May 2021, 10:26 > I agree, but sometimes feel it helps readability. Reply by pmontazemi on 04 May 2021, 10:31 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 03 May 2021, 08:55 https://devapps.diality.us/cru/HD-DEN-7605-2#c9683 Should all these alarms be using persistent alarm module from fwcommon? Using persistent alarm module will save some dev test time later on. Reply by Sean Nash on 03 May 2021, 10:23 > Yes, you are right. I will use fwcommon code. Reply by qnguyen on 04 May 2021, 10:38 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/Dialysis.c Revision Comment by qnguyen on 03 May 2021, 09:08 https://devapps.diality.us/cru/HD-DEN-7605-2#c9685 Add blank line between declaration and code. Reply by Sean Nash on 03 May 2021, 10:20 > What declaration? Reply by qnguyen on 04 May 2021, 10:37 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/BloodPrime.c Revision Comment by qnguyen on 05 May 2021, 19:57 https://devapps.diality.us/cru/HD-DEN-7605-2#c9759 Blood prime last motor count does not seem to get updated except in initBloodPrime(). Should it be updated continuously somewhere? Reply by Sean Nash on 06 May 2021, 10:27 > initBloodPrime() is called every time we transition to blood > prime sub-mode. Reply by qnguyen on 18 May 2021, 10:39 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/AlarmMgmt.c Revision Comment by qnguyen on 04 June 2021, 10:31 https://devapps.diality.us/cru/HD-DEN-7605-2#c10207 Should we call setAlarmAudio here? This test is running at 50 ms while the setAlarmAudio is running at 250 ms. The alarm audio might not be set when we switch to else part. Reply by Sean Nash on 07 June 2021, 09:26 > Good point. Test will need to be longer than 50ms anyway - > FPGA ramps the sound and there is a ~250ms time constant in > the electronics. But I will want the sound to at least get > started immediately. Reply by qnguyen on 07 June 2021, 10:32 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 03 May 2021, 10:44 https://devapps.diality.us/cru/HD-DEN-7605-2#c9704 These functions need doxygen description. Reply by Sean Nash on 03 May 2021, 11:40 > Fixed. Reply by qnguyen on 04 May 2021, 10:37 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 03 May 2021, 10:46 https://devapps.diality.us/cru/HD-DEN-7605-2#c9705 Why do we need to subtract from MAX_ALARM_VOLUME_LEVEL? Reply by Sean Nash on 03 May 2021, 11:37 > Copy and paste error. Fixed. Reply by qnguyen on 04 May 2021, 10:36 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 03 May 2021, 10:47 https://devapps.diality.us/cru/HD-DEN-7605-2#c9706 Same as above. Reply by Sean Nash on 03 May 2021, 11:38 > Fixed. Reply by qnguyen on 04 May 2021, 10:36 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/BloodFlow.c Revision Comment by qnguyen on 07 May 2021, 00:06 https://devapps.diality.us/cru/HD-DEN-7605-2#c9799 Same as comment from DialinFlow.c Reply by Sean Nash on 11 May 2021, 14:54 > Is this still an issue? Reply by qnguyen on 18 May 2021, 10:40 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 07 May 2021, 00:07 https://devapps.diality.us/cru/HD-DEN-7605-2#c9800 Need to remove this comment. Reply by Sean Nash on 11 May 2021, 14:53 > Agreed. This is for me to add a check for this error. I'll > remove comment when I'm done. Reply by qnguyen on 28 May 2021, 10:33 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: RTC.c Revision Comment by pmontazemi on 03 May 2021, 09:59 https://devapps.diality.us/cru/HD-DEN-7605-2#c9686 Fix which test to get 1000 ms? Reply by Sean Nash on 03 May 2021, 10:18 > Dara to refactor the POST test for RTC vs. processor timer so > that we expect 1000 ms of processor time after RTC second > changes. He and I have discussed this needed change. Reply by qnguyen on 18 May 2021, 10:42 > This will be addressed in DEN-8103. Reply by pmontazemi on 27 May 2021, 10:31 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/DialInFlow.c Revision Comment by qnguyen on 07 May 2021, 00:01 https://devapps.diality.us/cru/HD-DEN-7605-2#c9798 We can't have 3 calls to isPersistentAlarmTriggered() on same alarm. If the first call starts the timer and the second or third call is good, it will reset the timer to 0 again. Suggest creating a BOOL variable to have all three conditions or with each other and call isPersistentAlarmTriggered() only once. Reply by Sean Nash on 11 May 2021, 14:53 > Is this still an issue? Reply by qnguyen on 18 May 2021, 10:40 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 07 May 2021, 00:00 https://devapps.diality.us/cru/HD-DEN-7605-2#c9797 Need to remove this comment. Reply by Sean Nash on 11 May 2021, 14:54 > Agreed. This is for me to add a check for this error. Will > remove comment when I'm done. Reply by qnguyen on 28 May 2021, 10:34 > RESOLVED in CODE WALKTHROUGH. --- ID: HD-DEN-7605-2 https://devapps.diality.us/cru/HD-DEN-7605-2 Title: HD-DEN-7605_HD DG Self Tests Statement of Objectives: State: Closed Summary: Author: Sean Nash Moderator: Sean Nash Reviewers: (0 active, 3 completed*) qnguyen (*) Dara Navaei (*) pmontazemi (*)