This is a list of all comments for HD-DEN-13834-1. Review Summary: No summary ---------------------------------------- File: firmware/App/Controllers/BloodLeak.c Revision Comment by wbracken on 20 September 2022, 17:19 https://devapps.diality.us/cru/HD-DEN-13834-1#c13834 Add bloodLeakEmbModeHasZeroBeenRqustd to header. Reply by Dara Navaei on 29 September 2022, 09:33 > Done. Reply by wbracken on 17 October 2022, 11:00 > RESOLVED IN CODE WALKTHROUGH Revision Comment by wbracken on 17 October 2022, 14:50 https://devapps.diality.us/cru/HD-DEN-13834-1#c14453 Are bloodLeakEmbModeHasZeroBeenRqustd and bloodLeakSelfTestStatus both global? Reply by Dara Navaei on 17 October 2022, 15:08 > Added them to the doxygen comments. Reply by wbracken on 18 October 2022, 11:27 > RESOLVED IN CODE WALKTHROUGH ---------------------------------------- File: firmware/App/HDCommon.h Revision Comment by Michael Garthwaite on 30 September 2022, 09:00 https://devapps.diality.us/cru/HD-DEN-13834-1#c14075 Do we want this checked into staging? Reply by Dara Navaei on 14 October 2022, 10:11 > Disabled the #define. Reply by Michael Garthwaite on 14 October 2022, 10:57 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/DGInterface.c Revision Comment by wbracken on 28 September 2022, 22:13 https://devapps.diality.us/cru/HD-DEN-13834-1#c14037 Update function header. Reply by Dara Navaei on 29 September 2022, 09:30 > Done. Reply by wbracken on 17 October 2022, 10:59 > RESOLVED IN CODE WALKTHROUGH Revision Comment by wbracken on 28 September 2022, 22:14 https://devapps.diality.us/cru/HD-DEN-13834-1#c14038 Update function header. Reply by Dara Navaei on 29 September 2022, 09:28 > Done. Reply by wbracken on 17 October 2022, 10:54 > RESOLVED IN CODE WALKTHROUGH Revision Comment by wbracken on 28 September 2022, 22:15 https://devapps.diality.us/cru/HD-DEN-13834-1#c14039 Update function header. dgReservoirsDataFresFlag. Reply by Dara Navaei on 29 September 2022, 09:27 > Done. Reply by wbracken on 17 October 2022, 10:54 > RESOLVED IN CODE WALKTHROUGH Revision Comment by wbracken on 28 September 2022, 22:16 https://devapps.diality.us/cru/HD-DEN-13834-1#c14040 Update function header. dgDialysateFlowDataFresFlag. Reply by Dara Navaei on 29 September 2022, 09:26 > Done. Reply by wbracken on 17 October 2022, 10:54 > RESOLVED IN CODE WALKTHROUGH Revision Comment by wbracken on 28 September 2022, 22:16 https://devapps.diality.us/cru/HD-DEN-13834-1#c14041 Update function header. Reply by Dara Navaei on 29 September 2022, 09:25 > Done. Reply by wbracken on 17 October 2022, 10:52 > RESOLVED IN CODE WALKTHROUGH Revision Comment by wbracken on 28 September 2022, 22:19 https://devapps.diality.us/cru/HD-DEN-13834-1#c14044 What is this comment? Reply by Dara Navaei on 29 September 2022, 09:21 > The comment says the alarms are implemented per Product > Requirement Specifications. Reply by wbracken on 17 October 2022, 10:53 > RESOLVED IN CODE WALKTHROUGH Revision Comment by wbracken on 28 September 2022, 22:20 https://devapps.diality.us/cru/HD-DEN-13834-1#c14045 Add trimmerState. Reply by Dara Navaei on 29 September 2022, 09:21 > Done. Reply by wbracken on 17 October 2022, 10:53 > RESOLVED IN CODE WALKTHROUGH Revision Comment by Darren Cox on 14 October 2022, 12:08 https://devapps.diality.us/cru/HD-DEN-13834-1#c14410 Put define on left of comparison. DG_MODE_HEAT != dgOp. Reply by Dara Navaei on 17 October 2022, 11:51 > We only put the define on the left of the comparison is "==" > just to make sure if we put only one "=", the compiler will > catch it. Reply by Sean Nash on 18 October 2022, 11:25 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/DGInterface.h Revision Comment by wbracken on 19 September 2022, 15:20 https://devapps.diality.us/cru/HD-DEN-13834-1#c13815 Should struct members be aligned? Reply by Dara Navaei on 18 October 2023, 21:07 > RESOLVED IN CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Modes/BloodPrime.c Revision Comment by Sean Nash on 14 October 2022, 11:27 https://devapps.diality.us/cru/HD-DEN-13834-1#c14402 Can we uncomment this now? Reply by Dara Navaei on 17 October 2022, 13:14 > It is uncommented. Reply by Sean Nash on 18 October 2022, 11:22 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/SystemCommMessages.c Revision Comment by Sean Nash on 04 October 2022, 17:52 https://devapps.diality.us/cru/HD-DEN-13834-1#c14123 Keep blank line. Reply by Dara Navaei on 14 October 2022, 10:10 > Done. Reply by Sean Nash on 18 October 2022, 11:21 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 04 October 2022, 17:53 https://devapps.diality.us/cru/HD-DEN-13834-1#c14124 Why not checking payload length? Reply by Dara Navaei on 17 October 2022, 13:17 > Done. Reply by Sean Nash on 18 October 2022, 11:22 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: NVDataMgmt.c Revision Comment by Darren Cox on 19 September 2022, 15:27 https://devapps.diality.us/cru/HD-DEN-13834-1#c13819 Should this be #elseif? Can _DG_ AND _HD_ both be defined or neither? Reply by Dara Navaei on 29 September 2022, 09:37 > Both cannot be and are not defined at the same time. Reply by Sean Nash on 29 September 2022, 17:17 > These are defined in the Code Composer project for DG and > HD firmware respectively. You can only have one defined > and you must have one defined. Reply by Dara Navaei on 14 October 2022, 10:25 > This is in fwcommon so it has to be able to cover both > stacks. Reply by Sean Nash on 18 October 2022, 11:26 > RESOLVED in CODE WALKTHROUGH. Revision Comment by wbracken on 19 September 2022, 10:07 https://devapps.diality.us/cru/HD-DEN-13834-1#c13793 How is the event calibration update record being used? Reply by Dara Navaei on 18 October 2023, 21:07 > RESOLVED IN CODE WALKTHROUGH Revision Comment by wbracken on 19 September 2022, 10:13 https://devapps.diality.us/cru/HD-DEN-13834-1#c13795 Is this indentation correct? Reply by Dara Navaei on 29 September 2022, 09:42 > It seems correct to me. Reply by wbracken on 17 October 2022, 11:03 > RESOLVED IN CODE WALKTHROUGH Revision Comment by Sean Nash on 13 October 2022, 08:51 https://devapps.diality.us/cru/HD-DEN-13834-1#c14338 This looked better before. Why change? Reply by Dara Navaei on 14 October 2022, 09:35 > This was changed to be consistent with the rest of the FAPI > if statements. Reply by Sean Nash on 14 October 2022, 11:01 > RESOLVED in CODE WALKTHROUGH. Revision Comment by wbracken on 28 September 2022, 10:32 https://devapps.diality.us/cru/HD-DEN-13834-1#c14018 Should be hdUsageInfoGroup in header. Reply by Dara Navaei on 29 September 2022, 09:32 > Done. Reply by wbracken on 17 October 2022, 10:59 > RESOLVED IN CODE WALKTHROUGH ---------------------------------------- File: Utilities.c Revision Comment by wbracken on 19 September 2022, 10:39 https://devapps.diality.us/cru/HD-DEN-13834-1#c13796 Does this need to handle lowercase hex characters and invalid characters? Reply by Dara Navaei on 14 October 2022, 09:44 > Done. Reply by wbracken on 17 October 2022, 11:02 > RESOLVED IN CODE WALKTHROUGH Revision Comment by Sean Nash on 12 October 2022, 09:12 https://devapps.diality.us/cru/HD-DEN-13834-1#c14201 Is this a TODO? Can we just do it? Reply by Dara Navaei on 14 October 2022, 09:34 > Done. Reply by Sean Nash on 14 October 2022, 11:01 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Darren Cox on 14 October 2022, 12:06 https://devapps.diality.us/cru/HD-DEN-13834-1#c14409 Should a status of FALSE be set here since the input string is not a valid number? Reply by Dara Navaei on 17 October 2022, 11:53 > The status of FALSE is set at the top when the variable is > declared. If the conversion was successful, the status is set > to TRUE. Reply by Sean Nash on 18 October 2022, 11:25 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: RTC.c Revision Comment by Sean Nash on 12 October 2022, 09:11 https://devapps.diality.us/cru/HD-DEN-13834-1#c14200 Why are we checking non-zero here? Seems like we will get stuck here if years really is set to zero. Reply by Dara Navaei on 12 October 2022, 13:13 > Done. Reply by Sean Nash on 18 October 2022, 11:22 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: AlarmDefs.h Revision Comment by wbracken on 03 October 2022, 10:43 https://devapps.diality.us/cru/HD-DEN-13834-1#c14085 Are all available ALARM_IDs being removed? Reply by Sean Nash on 04 October 2022, 17:21 > Idea is to re-purpose these (instead of adding new alarms to > bottom of enum). We prefer not to collapse the table when an > alarm ID goes obsolete so other alarm IDs do not change. Reply by Dara Navaei on 18 October 2023, 21:13 > RESOLVED in CODE WALKTHROUGH Revision Comment by Sean Nash on 29 September 2022, 16:40 https://devapps.diality.us/cru/HD-DEN-13834-1#c14066 Faults (HD and DG) should probably be consistent on clear immediate concept. I think faults should be TRUE on clear immediate (going to fault mode anyway, so alarm condition isn't relevant after that). Reply by Dara Navaei on 12 October 2022, 13:36 > Done. Reply by Sean Nash on 13 October 2022, 23:58 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 29 September 2022, 16:39 https://devapps.diality.us/cru/HD-DEN-13834-1#c14063 No stop property for a high priority alarm? PRS 754 says dialysate temperature alarms are low priority. Temp too low = rank 902. Temp too high = rank 901. Reply by Dara Navaei on 12 October 2022, 13:38 > Done. The ranks will be redefined. Reply by Sean Nash on 14 October 2022, 11:00 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 29 September 2022, 16:43 https://devapps.diality.us/cru/HD-DEN-13834-1#c14068 No stop property for a high priority alarm? Reply by Dara Navaei on 12 October 2022, 13:31 > Done. Reply by Sean Nash on 13 October 2022, 23:58 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 29 September 2022, 16:46 https://devapps.diality.us/cru/HD-DEN-13834-1#c14069 For DG faults, let's be consistent. DG fault = TRUE Clear Immediate = TRUE (alarm condition moot because DG going to fault mode) Stops = TRUE (HD pauses treatment for alarm) No Clear = FALSE (so HD can clear alarm when user selects option) No Resume = TRUE (cannot resume treatment w/o functioning DG) No Rinseback = FALSE (no reason to prevent patient getting their blood back just because DG failed) No End Treatment = FALSE (should try to end treatment as normally as possible w/o DG - won't be able to drain reservoirs though) No Blood Recirc = FALSE (why not recirc blood?) No Dialysate Recirc = TRUE (may not be possible depending on what's wrong w/ DG) Clear Only = FALSE Reply by Dara Navaei on 12 October 2022, 13:30 > Done. Reply by Sean Nash on 13 October 2022, 23:57 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 29 September 2022, 16:53 https://devapps.diality.us/cru/HD-DEN-13834-1#c14070 Why no stop? Reply by Dara Navaei on 12 October 2022, 13:29 > Done. Reply by Sean Nash on 13 October 2022, 23:56 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 29 September 2022, 16:54 https://devapps.diality.us/cru/HD-DEN-13834-1#c14071 Why no stop? Reply by Dara Navaei on 12 October 2022, 13:30 > Done. Reply by Sean Nash on 13 October 2022, 23:56 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: DGDefs.h Revision Comment by Sean Nash on 29 September 2022, 17:06 https://devapps.diality.us/cru/HD-DEN-13834-1#c14072 What is the purpose of this "no event" event? Reply by Dara Navaei on 12 October 2022, 13:26 > Removed the "no event" event. Reply by Sean Nash on 18 October 2022, 11:27 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: HDDefs.h Revision Comment by Sean Nash on 29 September 2022, 17:06 https://devapps.diality.us/cru/HD-DEN-13834-1#c14073 What is the purpose of this "no event" event? Reply by Dara Navaei on 12 October 2022, 13:24 > This event is used for NV data management events. > HD_EVENT_NO_EVENT is not currently in use. Reply by Sean Nash on 13 October 2022, 08:55 > I don't understand. It's used by nv data but it's not > currently used? This enum is supposed to an enum of events > that can be sent to UI for logging. Why would we want to > log a non-event? Reply by Dara Navaei on 17 October 2022, 13:48 > Removed it. Reply by Sean Nash on 18 October 2022, 11:26 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModeFault.c Revision Comment by Sean Nash on 12 October 2022, 09:16 https://devapps.diality.us/cru/HD-DEN-13834-1#c14202 This probably warrants a comment. Reply by Dara Navaei on 12 October 2022, 13:12 > Done. Reply by Sean Nash on 13 October 2022, 23:50 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 14 October 2022, 11:30 https://devapps.diality.us/cru/HD-DEN-13834-1#c14404 0.0F (pwm) param not in function - remove. Reply by Dara Navaei on 17 October 2022, 13:10 > This is an old code and it has been removed. Reply by Sean Nash on 18 October 2022, 11:23 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 14 October 2022, 11:30 https://devapps.diality.us/cru/HD-DEN-13834-1#c14405 0.0F (pwm) param not in function - remove. Reply by Dara Navaei on 17 October 2022, 13:11 > This is an old code and it has been removed. Reply by Sean Nash on 18 October 2022, 11:23 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 14 October 2022, 11:30 https://devapps.diality.us/cru/HD-DEN-13834-1#c14406 0.0F (pwm) param not in function - remove. Reply by Dara Navaei on 17 October 2022, 13:11 > This is an old code and it has been removed. Reply by Sean Nash on 18 October 2022, 11:24 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModePreTreat.c Revision Comment by wbracken on 19 September 2022, 15:23 https://devapps.diality.us/cru/HD-DEN-13834-1#c13816 Should 250 be #define? Reply by Dara Navaei on 29 September 2022, 09:40 > No this line is a temporary line until the pump control is > fixed so the 100 mL/min can be achieved. Reply by wbracken on 17 October 2022, 11:01 > RESOLVED IN CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Modes/ModeStandby.c Revision Comment by wbracken on 19 September 2022, 15:24 https://devapps.diality.us/cru/HD-DEN-13834-1#c13817 Should 365 be #define? Why changed from #define to static const U32? Reply by Dara Navaei on 29 September 2022, 09:39 > It is a const because we want it to be calculated upon > compilation and be kept in memory. #define does the > calculations every time it is called. Reply by wbracken on 17 October 2022, 11:01 > RESOLVED IN CODE WALKTHROUGH Revision Comment by wbracken on 19 September 2022, 15:28 https://devapps.diality.us/cru/HD-DEN-13834-1#c13820 Update header? Reply by Dara Navaei on 29 September 2022, 09:36 > It is up to date. Reply by wbracken on 17 October 2022, 11:01 > RESOLVED IN CODE WALKTHROUGH Revision Comment by wbracken on 19 September 2022, 15:28 https://devapps.diality.us/cru/HD-DEN-13834-1#c13821 Update header. Reply by Dara Navaei on 29 September 2022, 09:35 > It is up to date. Reply by wbracken on 17 October 2022, 11:00 > RESOLVED IN CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Services/FPGA.c Revision Comment by wbracken on 04 October 2022, 11:07 https://devapps.diality.us/cru/HD-DEN-13834-1#c14101 What is the significance of (OBSOLETE)? Applies to several blocks. Reply by Sean Nash on 04 October 2022, 17:46 > These FPGA registers are obsolete - h/w is no longer in our > design. Need to continue to maintain register space since > registers after these have not moved. You can think of these > obsolete registers as "reserved" for future purpose. > Dara, I do think we could rename these register fields to > something like Reserved1, Reserved2, ... Reply by Dara Navaei on 12 October 2022, 13:20 > I am waiting for HDD to be updated to reserved so I can > rename them. Reply by Sean Nash on 13 October 2022, 09:01 > Why can't we rename them ourselves? Let's just do it > now. Reply by Dara Navaei on 17 October 2022, 13:47 > Done. Reply by wbracken on 17 October 2022, 15:01 > RESOLVED IN CODE WALKTHROUGH ---------------------------------------- File: PersistentAlarm.c Revision Comment by wbracken on 03 October 2022, 10:48 https://devapps.diality.us/cru/HD-DEN-13834-1#c14086 fpgaPersistentAlarmGroup is an input. Reply by Dara Navaei on 12 October 2022, 13:21 > Done. Reply by wbracken on 17 October 2022, 10:52 > RESOLVED IN CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Modes/Prime.c Revision Comment by Sean Nash on 04 October 2022, 17:37 https://devapps.diality.us/cru/HD-DEN-13834-1#c14119 Why was blank line removed? Reply by Dara Navaei on 12 October 2022, 13:16 > Done. Reply by Sean Nash on 14 October 2022, 11:32 > Removed again? Reply by Dara Navaei on 17 October 2022, 11:54 > No. Reply by Sean Nash on 18 October 2022, 11:24 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Tasks/TaskGeneral.c Revision Comment by Sean Nash on 04 October 2022, 17:42 https://devapps.diality.us/cru/HD-DEN-13834-1#c14121 Monitor execs should be before operation modes exec. Reply by Dara Navaei on 12 October 2022, 13:16 > Done. Reply by Sean Nash on 13 October 2022, 23:53 > Still below operation modes exec. Reply by Dara Navaei on 17 October 2022, 13:18 > Done. Reply by Sean Nash on 18 October 2022, 11:26 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/Temperatures.c Revision Comment by Sean Nash on 04 October 2022, 17:29 https://devapps.diality.us/cru/HD-DEN-13834-1#c14118 Align "=". Reply by Dara Navaei on 12 October 2022, 13:18 > Done. Reply by Sean Nash on 13 October 2022, 23:54 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/PresOccl.c Revision Comment by Sean Nash on 04 October 2022, 17:27 https://devapps.diality.us/cru/HD-DEN-13834-1#c14117 Add another blank line here. Reply by Dara Navaei on 12 October 2022, 13:19 > Done. Reply by Sean Nash on 13 October 2022, 23:54 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/Dialysis.c Revision Comment by Sean Nash on 14 October 2022, 11:28 https://devapps.diality.us/cru/HD-DEN-13834-1#c14403 Space before end ) Reply by Dara Navaei on 17 October 2022, 13:12 > Done. Reply by Sean Nash on 18 October 2022, 11:23 > RESOLVED in CODE WALKTHROUGH. --- ID: HD-DEN-13834-1 https://devapps.diality.us/cru/HD-DEN-13834-1 Title: HD-DEN-13834_DG HD Dev HD DG Dvt Update Part 3 Statement of Objectives: State: Closed Summary: Author: Dara Navaei Moderator: Dara Navaei Reviewers: (2 active, 2 completed*) Sean Nash (*) wbracken (*) Michael Garthwaite Darren Cox