This is a list of all comments for HD-DEN-8103-1. Review Summary: No summary ---------------------------------------- File: firmware/App/Modes/ModePostTreat.c Revision Comment by Sean Nash on 19 May 2021, 15:41 https://devapps.diality.us/cru/HD-DEN-8103-1#c9967 It doesn't look like this function sends anything to UI - just setting an epoch. Does f/w need to log treatment start/end time or can UI do that from its own clock. Reply by qnguyen on 20 May 2021, 11:13 > Removed. UI will be the logger, so the clock comes from only > one source. Reply by Sean Nash on 21 May 2021, 10:37 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 19 May 2021, 15:45 https://devapps.diality.us/cru/HD-DEN-8103-1#c9969 Need to add a get function for private variable totalSalineVolumeDelivered in Dialysis module. Reply by qnguyen on 19 May 2021, 16:38 > Done. Reply by Sean Nash on 21 May 2021, 10:37 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 19 May 2021, 15:43 https://devapps.diality.us/cru/HD-DEN-8103-1#c9968 This should be available from syringe pump driver - getSyringePumpVolumeDelivered(). Reply by qnguyen on 19 May 2021, 16:38 > Done. Reply by Sean Nash on 21 May 2021, 10:37 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/SampleWater.c Revision Comment by Sean Nash on 03 June 2021, 09:01 https://devapps.diality.us/cru/HD-DEN-8103-1#c10186 Shouldn't this be checked more broadly (not just in water sample sub-mode)? Reply by qnguyen on 03 June 2021, 10:23 > Yes, and we checked for DG restarted (after it has started by > HD) in DGInterface. > Sample water sub-mode has to check before DG started. Reply by Sean Nash on 03 June 2021, 10:32 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/AlarmMgmt.c Revision Comment by Sean Nash on 03 June 2021, 09:03 https://devapps.diality.us/cru/HD-DEN-8103-1#c10187 Is this different than alarmStatus.noNewTreatment? Reply by qnguyen on 03 June 2021, 10:54 > Removed this extra flag. Reply by Sean Nash on 07 June 2021, 10:34 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 03 June 2021, 09:06 https://devapps.diality.us/cru/HD-DEN-8103-1#c10188 Looks like you are latching the noEndTreatment status. Maybe we can have that status be latching so we don't need two flags. Reply by qnguyen on 03 June 2021, 10:55 > Removed this extra flag and used alarm status noNewTreatment > flag. Reply by Sean Nash on 04 June 2021, 10:15 > But now in order to use the status flag, I think we can't > allow that flag to go from TRUE to FALSE - need to latch > it. Reply by qnguyen on 04 June 2021, 10:18 > The flag is latched by using OR with the local updated > noNewTreatment status. Reply by Sean Nash on 07 June 2021, 10:34 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/AlarmMgmt.h Revision Comment by Sean Nash on 19 May 2021, 16:48 https://devapps.diality.us/cru/HD-DEN-8103-1#c9976 This enum is getting big. Should we put it in its own .h file (AlarmMgmtSWFaults.h) with same doxygen group name (AlarmManagement) that would be included by this header? Reply by qnguyen on 19 May 2021, 17:18 > Done. Reply by Sean Nash on 21 May 2021, 10:35 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModePostTreat.h Revision Comment by Sean Nash on 19 May 2021, 15:39 https://devapps.diality.us/cru/HD-DEN-8103-1#c9966 Do we not need concentration? Reply by qnguyen on 19 May 2021, 16:09 > The UI will provide this. Firmware will not have this value. Reply by Sean Nash on 21 May 2021, 10:38 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/Integrity.c Revision Comment by Sean Nash on 19 May 2021, 16:51 https://devapps.diality.us/cru/HD-DEN-8103-1#c9977 How long does this take? We may need to break this loop into smaller pieces and have ModeInitPOST call this function multiple times so that general task does not take too long (> 40ms or so). Otherwise, watchdog will not get pet within its ~50ms window and expire - and also other general and background operations will be blocked until this test is completed. Reply by qnguyen on 20 May 2021, 11:12 > It took around 55 ms for the largest section (.text). I have > imposed a maximum size per each CRC calculation, which takes > around 7 ms each. Reply by Sean Nash on 21 May 2021, 10:35 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModeTreatment.c Revision Comment by Sean Nash on 19 May 2021, 16:30 https://devapps.diality.us/cru/HD-DEN-8103-1#c9972 Neither of these if statements should be executed if treatment is paused (i.e. treatment time is counting down, i.e. we're in dialysis sub-mode of treatment mode and not delivering a saline bolus). Reply by qnguyen on 20 May 2021, 12:14 > Changed to use treatmentTimeMS to increase the log time. Reply by Sean Nash on 21 May 2021, 10:37 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 19 May 2021, 16:40 https://devapps.diality.us/cru/HD-DEN-8103-1#c9975 For UF, need to accumulate volume deltas or remember where 30 min volume started and then subtract from current volume when 30 minutes is up. Then convert to rate by dividing volume by 30 min. Reply by qnguyen on 19 May 2021, 17:18 > Done. Reply by Sean Nash on 21 May 2021, 10:35 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 19 May 2021, 16:29 https://devapps.diality.us/cru/HD-DEN-8103-1#c9971 Let UI timestamp Reply by qnguyen on 19 May 2021, 17:18 > Removed. Reply by Sean Nash on 21 May 2021, 10:37 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/SystemCommMessages.h Revision Comment by Sean Nash on 19 May 2021, 16:57 https://devapps.diality.us/cru/HD-DEN-8103-1#c9978 Change PERIOD to PERIODIC? Reply by qnguyen on 19 May 2021, 17:12 > Done. Reply by Sean Nash on 21 May 2021, 10:34 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 23 June 2021, 09:28 https://devapps.diality.us/cru/HD-DEN-8103-1#c10311 Why removed? Reply by qnguyen on 23 June 2021, 10:07 > Duplicate declaration from line 330. Reply by Sean Nash on 25 June 2021, 10:34 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: AlarmDefs.h Revision Comment by Sean Nash on 28 May 2021, 15:27 https://devapps.diality.us/cru/HD-DEN-8103-1#c10139 I think rather than 37, these should be the lowest high priority rank (i.e. 49) so we can add new alarms and not have to renumber these. Also, I think the range for high priority alarms should have room for more than 50 alarms. Maybe high priority could be 1..499, medium priority 500..599, and low priority 600..699. Reply by qnguyen on 03 June 2021, 10:51 > Done. Reply by Sean Nash on 07 June 2021, 10:34 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: RTC.c Revision Comment by Sean Nash on 28 May 2021, 15:23 https://devapps.diality.us/cru/HD-DEN-8103-1#c10137 We should delete this dead code as well. Reply by qnguyen on 29 May 2021, 01:06 > Removed. Reply by Sean Nash on 03 June 2021, 10:33 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 28 May 2021, 15:23 https://devapps.diality.us/cru/HD-DEN-8103-1#c10138 Dara, do we need these handlers or not? Reply by qnguyen on 07 June 2021, 10:34 > Per [~dnavaei], we will need to implement this functions. Reply by Sean Nash on 07 June 2021, 10:35 > RESOLVED in CODE WALKTHROUGH. Revision Comment by pmontazemi on 21 June 2021, 13:58 https://devapps.diality.us/cru/HD-DEN-8103-1#c10301 Add empty line before return. Reply by qnguyen on 23 June 2021, 10:08 > Done. Reply by pmontazemi on 25 June 2021, 10:35 > RESOLVED in CODE WALKTHROUGH. --- ID: HD-DEN-8103-1 https://devapps.diality.us/cru/HD-DEN-8103-1 Title: HD-DEN-8103_DG HD Dev Alarm Design Statement of Objectives: State: Closed Summary: Author: qnguyen Moderator: qnguyen Reviewers: (0 active, 3 completed*) Sean Nash (*) Dara Navaei (*) pmontazemi (*)