This is a list of all comments for HD-DEN-7117-1. Review Summary: No summary ---------------------------------------- File: firmware/App/Controllers/BloodFlow.c Revision Comment by pmontazemi on 22 March 2021, 17:23 https://devapps.diality.us/cru/HD-DEN-7117-1#c8537 Not better to use this instead? bloodPumpMotorEdgeCount += (U16)(delta); Reply by Sean Nash on 22 March 2021, 22:43 > Agreed. Tricky because delta is signed, but it works the way > I need it to. Reply by pmontazemi on 26 March 2021, 10:36 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/SyringePump.c Revision Comment by pmontazemi on 01 April 2021, 10:42 https://devapps.diality.us/cru/HD-DEN-7117-1#c8920 Use 10 instead of ten (easier to track and compare). Reply by Sean Nash on 02 April 2021, 09:49 > Compiler will not allow numeric start to a #define name. Reply by pmontazemi on 02 April 2021, 10:39 > I am referring to the comment. Reply by Sean Nash on 06 April 2021, 09:21 > Done. Reply by pmontazemi on 06 April 2021, 10:24 > RESOLVED in CODE WALKTHROUGH. Revision Comment by pmontazemi on 01 April 2021, 10:42 https://devapps.diality.us/cru/HD-DEN-7117-1#c8921 Use 5 instead of five (easier to track and compare). Reply by Sean Nash on 02 April 2021, 09:50 > Compiler will not allow numeric start to a #define name. Reply by pmontazemi on 02 April 2021, 10:39 > I am referring to the comment. Reply by Sean Nash on 06 April 2021, 09:21 > Done. Reply by pmontazemi on 06 April 2021, 10:24 > RESOLVED in CODE WALKTHROUGH. Revision Comment by pmontazemi on 22 March 2021, 17:30 https://devapps.diality.us/cru/HD-DEN-7117-1#c8538 Either add TODO or delete commented line. Reply by Sean Nash on 22 March 2021, 22:35 > Added TODO for now. Reply by pmontazemi on 26 March 2021, 10:37 > RESOLVED in CODE WALKTHROUGH. Revision Comment by pmontazemi on 01 April 2021, 10:43 https://devapps.diality.us/cru/HD-DEN-7117-1#c8922 Replace 0x80 with constant and define it at beginning of *.c file. Reply by Sean Nash on 02 April 2021, 08:46 > Done. Reply by pmontazemi on 02 April 2021, 10:39 > RESOLVED in CODE WALKTHROUGH. Revision Comment by pmontazemi on 01 April 2021, 10:43 https://devapps.diality.us/cru/HD-DEN-7117-1#c8923 Replace 0x3F with constant and define it at beginning of *.c file. Reply by Sean Nash on 02 April 2021, 08:44 > Done. Reply by pmontazemi on 02 April 2021, 10:40 > RESOLVED in CODE WALKTHROUGH. Revision Comment by pmontazemi on 23 March 2021, 13:25 https://devapps.diality.us/cru/HD-DEN-7117-1#c8644 What does the V stand for in the name of this function? Can we be more explicit? If voltage, then "Volt". Reply by Sean Nash on 23 March 2021, 14:02 > It is volts. Everywhere else we use abbreviated units of > measure as suffix (e.g. _ML). Reply by pmontazemi on 26 March 2021, 10:38 > RESOLVED in CODE WALKTHROUGH. Revision Comment by pmontazemi on 01 April 2021, 10:43 https://devapps.diality.us/cru/HD-DEN-7117-1#c8924 Replace 0x40 with constant and define it at beginning of *.c file. Reply by Sean Nash on 02 April 2021, 08:42 > Done. Reply by pmontazemi on 02 April 2021, 10:40 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 29 March 2021, 22:20 https://devapps.diality.us/cru/HD-DEN-7117-1#c8882 Should this be an or? Reply by Sean Nash on 01 April 2021, 08:50 > I think this should be an and because of the "whichever is > greater" part of the condition. Reply by qnguyen on 02 April 2021, 10:32 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 29 March 2021, 22:20 https://devapps.diality.us/cru/HD-DEN-7117-1#c8883 Should this be an or? Reply by Sean Nash on 01 April 2021, 08:47 > I think it should be and. Because it is a "whichever is > greater" condition. So if error is 6% but delta is less than > 0.1 mL, I would not want to trigger the error because the > volume is too small to alarm on a percentage basis. > Likewise, if delta is > 0.1 mL but error is < 5%, no alarm. Reply by qnguyen on 02 April 2021, 10:32 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/Dialysis.c Revision Comment by qnguyen on 17 March 2021, 14:45 https://devapps.diality.us/cru/HD-DEN-7117-1#c8380 Suggest combining into one line. Reply by Sean Nash on 19 March 2021, 09:44 > Done. Reply by qnguyen on 19 March 2021, 10:36 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 17 March 2021, 14:48 https://devapps.diality.us/cru/HD-DEN-7117-1#c8381 Why is UF off state goes back to UF running state right away? Should we remove this state then? Reply by Sean Nash on 19 March 2021, 09:42 > UF off and complete states are going to be removed. Removed > case handlers and handler functions. Keeping enums until UI > is updated to not use them. Reply by qnguyen on 19 March 2021, 10:36 > RESOLVED in CODE WALKTHROUGH. Revision Comment by pmontazemi on 06 April 2021, 08:04 https://devapps.diality.us/cru/HD-DEN-7117-1#c9012 Spell out lc variable as loadCell. Reply by Sean Nash on 06 April 2021, 09:01 > Done. Reply by pmontazemi on 06 April 2021, 10:24 > RESOLVED in CODE WALKTHROUGH. Revision Comment by pmontazemi on 23 March 2021, 13:11 https://devapps.diality.us/cru/HD-DEN-7117-1#c8643 Are we getting load cell weight in other units somewhere else? If not, I would suggest defining the units in the function header and removing it from the name of the function. Reply by Sean Nash on 23 March 2021, 14:05 > Done. Reply by pmontazemi on 26 March 2021, 10:37 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/SystemCommMessages.c Revision Comment by qnguyen on 05 April 2021, 16:54 https://devapps.diality.us/cru/HD-DEN-7117-1#c9007 This seems to be an incorrect message ID. Reply by Sean Nash on 06 April 2021, 09:19 > Good catch. Fixed. Reply by qnguyen on 06 April 2021, 10:20 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/FPGA.c Revision Comment by qnguyen on 17 March 2021, 14:43 https://devapps.diality.us/cru/HD-DEN-7117-1#c8379 Need doxygen description for new functions. Reply by Sean Nash on 19 March 2021, 09:56 > Done. Reply by qnguyen on 19 March 2021, 10:36 > RESOLVED in CODE WALKTHROUGH. Revision Comment by pmontazemi on 06 April 2021, 08:53 https://devapps.diality.us/cru/HD-DEN-7117-1#c9014 It would be helpful to say which syringe pump controller signal is being passed via all these channels instead of just a channel number. Reply by Sean Nash on 06 April 2021, 08:59 > Done. Reply by pmontazemi on 06 April 2021, 10:26 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/SystemComm.c Revision Comment by qnguyen on 05 April 2021, 17:05 https://devapps.diality.us/cru/HD-DEN-7117-1#c9008 This message should be MSG_ID_UI_INITIATE_TREATMENT_REQUEST. Reply by Sean Nash on 06 April 2021, 09:16 > Updated to match DEN-7091. Reply by qnguyen on 06 April 2021, 10:19 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModeFault.c Revision Comment by pmontazemi on 06 April 2021, 08:05 https://devapps.diality.us/cru/HD-DEN-7117-1#c9013 Comments start with capital letter. Reply by Sean Nash on 06 April 2021, 09:00 > Fixed. Reply by pmontazemi on 06 April 2021, 10:25 > RESOLVED in CODE WALKTHROUGH. --- ID: HD-DEN-7117-1 https://devapps.diality.us/cru/HD-DEN-7117-1 Title: HD-DEN-7117_HD Heparin Delivery Statement of Objectives: State: Closed Summary: Author: Sean Nash Moderator: Sean Nash Reviewers: (0 active, 3 completed*) qnguyen (*) Dara Navaei (*) pmontazemi (*)