This is a list of all comments for HD-DEN-4641-1. Review Summary: No summary ---------------------------------------- File: firmware/App/Controllers/PresOccl.c Revision Comment by pmontazemi on 01 December 2020, 13:19 https://devapps.diality.us/cru/HD-DEN-4641-1#c6555 How are we tracking TODO items? Do we search for both TODO before release or for both TODO and TBD? I believe we should stick to TBD only. Reply by Sean Nash on 01 December 2020, 13:30 > TODO is for reminders to do something later. TBD is for > something that is unknown for now. I would prefer keeping > them separate, but if I had to consolidate to one I would > rather use TODO. Reply by pmontazemi on 01 December 2020, 15:26 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 23 November 2020, 09:37 https://devapps.diality.us/cru/HD-DEN-4641-1#c6282 The condition of if statement can be passed directly into second parameter of checkPersistentAlarm function. This will remove the need for if and else. Reply by Sean Nash on 30 November 2020, 21:36 > Fixed. Reply by qnguyen on 01 December 2020, 10:35 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 23 November 2020, 09:38 https://devapps.diality.us/cru/HD-DEN-4641-1#c6283 same as above comment. Reply by Sean Nash on 30 November 2020, 21:36 > Fixed. Reply by qnguyen on 01 December 2020, 10:35 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 23 November 2020, 09:38 https://devapps.diality.us/cru/HD-DEN-4641-1#c6284 same as above comment. Reply by Sean Nash on 30 November 2020, 21:37 > Fixed. Reply by qnguyen on 01 December 2020, 10:35 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 23 November 2020, 09:40 https://devapps.diality.us/cru/HD-DEN-4641-1#c6285 change 0xCCC33C33 to OVERRIDE_KEY. Reply by Sean Nash on 30 November 2020, 21:32 > Done. Reply by qnguyen on 01 December 2020, 10:35 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/BloodFlow.c Revision Comment by pmontazemi on 01 December 2020, 14:31 https://devapps.diality.us/cru/HD-DEN-4641-1#c6586 No more flexibility on duration/frequency of this particular alarm needed? Reply by Sean Nash on 01 December 2020, 14:57 > Quang changed the way persistent alarms work (from task > interval counts to ms). So task interval is no longer needed > to determine when an alarm condition has persisted for too > long. Reply by pmontazemi on 01 December 2020, 15:27 > RESOLVED in CODE WALKTHROUGH. Revision Comment by pmontazemi on 24 November 2020, 09:48 https://devapps.diality.us/cru/HD-DEN-4641-1#c6304 Why double underscore between BUFFER and LEN? Reply by Sean Nash on 30 November 2020, 21:26 > Typo I guess. Fixed. Reply by pmontazemi on 01 December 2020, 10:33 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 23 November 2020, 10:00 https://devapps.diality.us/cru/HD-DEN-4641-1#c6287 Consider converting targetBloodFlowRate to non-override type and remove get function. Reply by Sean Nash on 30 November 2020, 21:44 > Done. Reply by qnguyen on 01 December 2020, 10:34 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/DialInFlow.c Revision Comment by Dara Navaei on 01 December 2020, 13:25 https://devapps.diality.us/cru/HD-DEN-4641-1#c6557 Why there are two __ before LEN? Reply by Sean Nash on 01 December 2020, 13:26 > Done. Reply by Dara Navaei on 01 December 2020, 15:34 > RESOLVED in CODE WALKTHROUGH. Revision Comment by pmontazemi on 01 December 2020, 09:12 https://devapps.diality.us/cru/HD-DEN-4641-1#c6488 if ( targetDialInFlowRate == 0 ) Reply by Sean Nash on 01 December 2020, 12:35 > When using "==" with a literal, I always put the literal > first so that if I accidentally use "=" the compiler will > throw a syntax error. Reply by pmontazemi on 01 December 2020, 15:01 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 23 November 2020, 09:56 https://devapps.diality.us/cru/HD-DEN-4641-1#c6286 Consider converting targetDialInFlowRate to non-override type and remove get function. Reply by Sean Nash on 30 November 2020, 21:52 > Done. Reply by qnguyen on 01 December 2020, 10:34 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/DialOutFlow.c Revision Comment by pmontazemi on 01 December 2020, 13:15 https://devapps.diality.us/cru/HD-DEN-4641-1#c6551 Comment mentions blood, is not this *.c file related to dialysate out pump? Reply by Sean Nash on 01 December 2020, 13:38 > Fixed. Reply by pmontazemi on 01 December 2020, 15:24 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Dara Navaei on 01 December 2020, 13:32 https://devapps.diality.us/cru/HD-DEN-4641-1#c6564 Recommend changing all the PWMs to duty cycle for consistency. Reply by Sean Nash on 01 December 2020, 13:51 > Done. Reply by Dara Navaei on 01 December 2020, 15:34 > RESOLVED in CODE WALKTHROUGH. Revision Comment by pmontazemi on 01 December 2020, 09:30 https://devapps.diality.us/cru/HD-DEN-4641-1#c6491 Move 2 characters to the left to align with two first lines. Reply by Sean Nash on 01 December 2020, 12:37 > Done. Reply by pmontazemi on 01 December 2020, 15:06 > RESOLVED in CODE WALKTHROUGH. Revision Comment by pmontazemi on 01 December 2020, 13:16 https://devapps.diality.us/cru/HD-DEN-4641-1#c6553 Remove extra line. Reply by Sean Nash on 01 December 2020, 13:34 > Done. Reply by pmontazemi on 01 December 2020, 15:25 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Dara Navaei on 01 December 2020, 13:34 https://devapps.diality.us/cru/HD-DEN-4641-1#c6566 An extra line is needed here. Reply by Sean Nash on 01 December 2020, 13:47 > Done. Reply by Dara Navaei on 01 December 2020, 15:33 > RESOLVED in CODE WALKTHROUGH. Revision Comment by pmontazemi on 01 December 2020, 13:17 https://devapps.diality.us/cru/HD-DEN-4641-1#c6554 Do we need to consider the case where ctrlMode is >= NUM_OF_PUMP_CONTROL_MODES? Reply by Sean Nash on 01 December 2020, 13:33 > In that case, the given mode is illegal and no action is > taken and result is returned FALSE. Dialin will see the > FALSE return and know it was unsuccessful. Reply by pmontazemi on 01 December 2020, 15:25 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/SystemCommMessages.c Revision Comment by qnguyen on 01 December 2020, 13:15 https://devapps.diality.us/cru/HD-DEN-4641-1#c6550 The function name is not matched. Reply by Sean Nash on 01 December 2020, 13:39 > Fixed. Reply by qnguyen on 01 December 2020, 15:29 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 01 December 2020, 13:16 https://devapps.diality.us/cru/HD-DEN-4641-1#c6552 Need to update parameters. Reply by Sean Nash on 01 December 2020, 13:36 > Fixed. Reply by qnguyen on 01 December 2020, 15:29 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/OperationModes.c Revision Comment by Dara Navaei on 01 December 2020, 13:53 https://devapps.diality.us/cru/HD-DEN-4641-1#c6576 Why is this #include in <> and not ""? Reply by Sean Nash on 01 December 2020, 14:03 > Don't know. Changed to "" for consistency. Reply by Dara Navaei on 01 December 2020, 15:30 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Dara Navaei on 01 December 2020, 13:54 https://devapps.diality.us/cru/HD-DEN-4641-1#c6578 Did we decide to separate the HALCoGen includes from the internal includes? Reply by Sean Nash on 01 December 2020, 14:04 > Yes. Fixed. Reply by Dara Navaei on 01 December 2020, 15:30 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/AlarmMgmt.h Revision Comment by pmontazemi on 01 December 2020, 09:32 https://devapps.diality.us/cru/HD-DEN-4641-1#c6492 push, 2 with space in between Reply by Sean Nash on 01 December 2020, 12:38 > Done. Reply by pmontazemi on 01 December 2020, 15:24 > RESOLVED in CODE WALKTHROUGH. Revision Comment by pmontazemi on 01 December 2020, 13:23 https://devapps.diality.us/cru/HD-DEN-4641-1#c6556 Why so many underscores? (there are 6 lines in this file having this issue) Reply by Sean Nash on 01 December 2020, 13:27 > These are enums that are available (I something there but > later they became obsolete). So I'm trying to call attention > to these so they will be used again. Reply by pmontazemi on 01 December 2020, 15:27 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: NVDataMgmt.c Revision Comment by pmontazemi on 24 November 2020, 09:46 https://devapps.diality.us/cru/HD-DEN-4641-1#c6303 Provide comment as to why Bank 7 Sector 0 is used here. Reply by Sean Nash on 30 November 2020, 21:30 > Done. Reply by pmontazemi on 01 December 2020, 10:33 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/DGInterface.h Revision Comment by Dara Navaei on 01 December 2020, 13:14 https://devapps.diality.us/cru/HD-DEN-4641-1#c6549 PWM should be changed to duty cycle. Reply by Sean Nash on 01 December 2020, 13:43 > Done. Reply by Dara Navaei on 01 December 2020, 15:35 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModeTreatment.c Revision Comment by qnguyen on 01 December 2020, 14:53 https://devapps.diality.us/cru/HD-DEN-4641-1#c6589 Missing param description. Reply by Sean Nash on 01 December 2020, 15:14 > Added @param. Reply by qnguyen on 01 December 2020, 15:28 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModeTreatmentParams.h Revision Comment by Dara Navaei on 01 December 2020, 13:50 https://devapps.diality.us/cru/HD-DEN-4641-1#c6573 Please comment the variables in the struct. Reply by Sean Nash on 01 December 2020, 14:00 > Done. Reply by Dara Navaei on 01 December 2020, 15:32 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Dara Navaei on 01 December 2020, 13:51 https://devapps.diality.us/cru/HD-DEN-4641-1#c6574 Please comment the variables in the structure. Reply by Sean Nash on 01 December 2020, 14:01 > Done. Reply by Dara Navaei on 01 December 2020, 15:31 > RESOLVED in CODE WALKTHROUGH. --- ID: HD-DEN-4641-1 https://devapps.diality.us/cru/HD-DEN-4641-1 Title: HD-DEN-4641_HD Firmware Occlusion & Pressure Sensors (2 of 3) Statement of Objectives: State: Closed Summary: Author: Sean Nash Reviewers: (0 active, 3 completed*) qnguyen (*) Dara Navaei (*) pmontazemi (*)