This is a list of all comments for DD-LEAH-1971-1. Review Summary: No summary ---------------------------------------- File: firmware/App/Controllers/ConcentratePumps.c Revision Comment by Sean Nash on 27 January 2025, 15:54 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21134 I believe this increment is for ramping. Would be helpful to have the word "ramp" somewhere in the name or at least in the comment. Also, indent on comment is out of alignment. Reply by Vinayakam Mani on 28 January 2025, 11:22 > Done. Revision Comment by Sean Nash on 27 January 2025, 15:56 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21135 Change 1 to 1.0F to make clear we are defining a floating point value (not integer). Reply by Vinayakam Mani on 28 January 2025, 11:22 > Done. Revision Comment by Sean Nash on 27 January 2025, 15:57 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21136 Remove extra blank line. Reply by Vinayakam Mani on 28 January 2025, 11:22 > Done. Revision Comment by Sean Nash on 27 January 2025, 16:01 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21138 This function is pretty large. Recommend moving some of this code to smaller static functions and calling them from the monitor function. Reply by Vinayakam Mani on 28 January 2025, 11:22 > Done. Revision Comment by Sean Nash on 27 January 2025, 16:00 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21137 Align "=" signs. Reply by Vinayakam Mani on 28 January 2025, 11:22 > Done. Revision Comment by Sean Nash on 27 January 2025, 16:08 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21139 Missing new targetVolume_ml param in header. Reply by Vinayakam Mani on 28 January 2025, 11:23 > Done. Revision Comment by Sean Nash on 27 January 2025, 16:14 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21140 Can we remove this line of code? Reply by Vinayakam Mani on 28 January 2025, 11:23 > Done. Revision Comment by Sean Nash on 27 January 2025, 16:14 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21141 Can we remove this line of code? Reply by Vinayakam Mani on 28 January 2025, 11:23 > Done. Revision Comment by Sean Nash on 27 January 2025, 16:15 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21142 In some places in this unit, I see an if/else structure (if is acid and else assumes bicarb) which is cleaner - no need for default with alarm. I'm ok with either approach, but we should be consistent throughout the unit. Reply by Vinayakam Mani on 28 January 2025, 11:23 > Done. ---------------------------------------- File: firmware/App/Controllers/DialysatePumps.c Revision Comment by Sean Nash on 27 January 2025, 16:24 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21143 Are we done with the 250 version of control interval? Can we remove? Reply by Vinayakam Mani on 28 January 2025, 11:23 > Done. Revision Comment by Sean Nash on 27 January 2025, 16:24 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21144 Mention "ramp" somewhere in name or at least comment. Reply by Vinayakam Mani on 28 January 2025, 11:23 > Done. Revision Comment by Sean Nash on 27 January 2025, 16:25 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21145 Why 1 and 0? Do these make sense? Reply by Vinayakam Mani on 28 January 2025, 11:24 > It will be evaluated/updated when close loop pump testing > happens. Revision Comment by Sean Nash on 27 January 2025, 16:28 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21146 Shouldn't we assign the minimum here (maybe that is 0.0, but we should use the #define). Reply by Vinayakam Mani on 28 January 2025, 11:25 > Done. Revision Comment by Sean Nash on 27 January 2025, 16:29 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21147 Remove extra blank line. Reply by Vinayakam Mani on 28 January 2025, 11:25 > Done. Revision Comment by Sean Nash on 27 January 2025, 16:30 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21148 Assume we re-activate this alarm code later so add a TODO comment. Reply by Vinayakam Mani on 28 January 2025, 11:25 > Done. Revision Comment by Sean Nash on 27 January 2025, 16:30 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21149 Add a TODO comment to reactivate this code. Reply by Vinayakam Mani on 28 January 2025, 11:25 > Done. Revision Comment by Sean Nash on 27 January 2025, 16:31 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21150 Add TODO comment. Reply by Vinayakam Mani on 28 January 2025, 11:25 > Done. Revision Comment by Sean Nash on 27 January 2025, 16:32 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21151 I don't think this comment is true in this case. The for loop is for pump ID and the switch is for pump state. Reply by Vinayakam Mani on 28 January 2025, 11:25 > Done. Reply by Sean Nash on 29 January 2025, 13:24 > I guess my point was about more than the comment. Can we > test the default case in vectorcast? I think we can and so > we should remove the #ifndef. Reply by Vinayakam Mani on 29 January 2025, 17:45 > Done. Revision Comment by Sean Nash on 27 January 2025, 16:33 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21152 Change 0 to 0.0F. Reply by Vinayakam Mani on 28 January 2025, 11:25 > Done. Revision Comment by Sean Nash on 27 January 2025, 16:34 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21153 Add TODO comment. Reply by Vinayakam Mani on 28 January 2025, 11:26 > Done. Revision Comment by Sean Nash on 27 January 2025, 16:34 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21154 Add TODO comment. Reply by Vinayakam Mani on 28 January 2025, 11:26 > Done. ---------------------------------------- File: firmware/App/Controllers/Heaters.c Revision Comment by Sean Nash on 27 January 2025, 16:42 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21155 Let's be consistent on how we handle multiple heaters. Here it's if/else but in other places it's a switch. Reply by Vinayakam Mani on 28 January 2025, 11:42 > Done. ---------------------------------------- File: firmware/App/Controllers/Valves.c Revision Comment by Sean Nash on 28 January 2025, 10:25 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21160 Can we remove this code now? Reply by Vinayakam Mani on 28 January 2025, 13:44 > Done. ---------------------------------------- File: firmware/App/Controllers/Valves.h Revision Comment by Sean Nash on 27 January 2025, 16:47 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21159 Remove extra blank line. Reply by Vinayakam Mani on 28 January 2025, 13:49 > Done. ---------------------------------------- File: firmware/App/Drivers/PressureSensor.c Revision Comment by Sean Nash on 28 January 2025, 10:51 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21161 Remove extra blank line. Reply by Vinayakam Mani on 28 January 2025, 13:52 > Done. ---------------------------------------- File: firmware/App/Drivers/TemperatureSensors.c Revision Comment by Sean Nash on 28 January 2025, 10:53 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21162 Should this be a build switch in DDCommon.h? Reply by Vinayakam Mani on 28 January 2025, 14:05 > Done. Revision Comment by Sean Nash on 28 January 2025, 10:53 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21163 Should these only be defined if USE_PT_100 is defined? Reply by Vinayakam Mani on 28 January 2025, 14:05 > Done. Revision Comment by Sean Nash on 28 January 2025, 10:55 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21164 Add TODO Reply by Vinayakam Mani on 28 January 2025, 14:12 > Done. ---------------------------------------- File: firmware/App/Modes/ModeGenDialysate.c Revision Comment by Sean Nash on 28 January 2025, 11:14 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21170 Treatment is misspelled. Reply by Vinayakam Mani on 28 January 2025, 14:21 > Done. Revision Comment by Sean Nash on 28 January 2025, 11:01 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21165 Misspelled Dialysate. Reply by Vinayakam Mani on 28 January 2025, 14:21 > Done. Revision Comment by Sean Nash on 28 January 2025, 11:03 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21166 Comment seems unnecessary. Reply by Vinayakam Mani on 28 January 2025, 14:24 > Done. Revision Comment by Sean Nash on 28 January 2025, 11:03 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21167 Comment no longer necessary. Reply by Vinayakam Mani on 28 January 2025, 14:24 > Done. Revision Comment by Sean Nash on 28 January 2025, 11:04 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21168 Should balancing chamber unit provide a function to set valves in one of the two states? Reply by Vinayakam Mani on 28 January 2025, 14:35 > Done. Revision Comment by Sean Nash on 28 January 2025, 11:18 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21173 Recommend just calling updateTreatmentSettings() always and putting the flag check/reset code in the function. Reply by Vinayakam Mani on 28 January 2025, 14:44 > Done. Revision Comment by Sean Nash on 28 January 2025, 11:15 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21171 Can we remove this line of code? Reply by Vinayakam Mani on 28 January 2025, 14:45 > Done. Revision Comment by Sean Nash on 28 January 2025, 11:17 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21172 Treatment is misspelled. Reply by Vinayakam Mani on 28 January 2025, 14:46 > Done. Revision Comment by Sean Nash on 28 January 2025, 11:20 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21174 Is there not a generic helper function that does this? getF32OverrideValue() in TestSupport.c? Reply by Vinayakam Mani on 28 January 2025, 14:56 > Done. Revision Comment by Sean Nash on 28 January 2025, 11:22 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21176 Why is this function commented out? Do we not need it? Reply by Vinayakam Mani on 28 January 2025, 15:22 > Done. Revision Comment by Sean Nash on 29 January 2025, 13:30 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21261 Add a blank line between function and end group marker. Reply by Vinayakam Mani on 29 January 2025, 17:29 > Done. ---------------------------------------- File: firmware/App/Modes/ModeStandby.c Revision Comment by Sean Nash on 28 January 2025, 11:26 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21197 There is no standby solo mode in Leahi. I think we would trigger a recoverable comm loss alarm. Reply by Vinayakam Mani on 28 January 2025, 15:28 > Done. Revision Comment by Sean Nash on 28 January 2025, 11:28 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21198 When would we get this request? Should it be handled by balancing chamber unit directly? Reply by Vinayakam Mani on 28 January 2025, 15:28 > This request is to switch balancing chamber without > pressure/dosing/heater/pumping functionality. the use case > for now is to do manual priming process. Will reevaluate > further as we develop more use cases. Revision Comment by Sean Nash on 28 January 2025, 11:30 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21199 When would this request be made? Reply by Vinayakam Mani on 28 January 2025, 15:30 > Same as above. ---------------------------------------- File: firmware/App/Monitors/Level.c Revision Comment by Sean Nash on 28 January 2025, 11:32 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21200 Consider putting this code in a static function. Reply by Vinayakam Mani on 28 January 2025, 15:45 > Done. Revision Comment by Sean Nash on 28 January 2025, 11:35 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21201 Why is this commented out? Can we remove it? Reply by Vinayakam Mani on 28 January 2025, 15:45 > Removed. ---------------------------------------- File: firmware/App/Services/AlarmMgmtSWFaults.h Revision Comment by Sean Nash on 28 January 2025, 12:55 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21205 I would change 105 to 9999 or something very high and then add any new s/w faults below these (i.e. 105..). Reply by Vinayakam Mani on 28 January 2025, 15:50 > Done. ---------------------------------------- File: firmware/App/Services/FpgaDD.c Revision Comment by Sean Nash on 28 January 2025, 13:08 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21216 Obsolete? Reply by Vinayakam Mani on 28 January 2025, 16:14 > Done Revision Comment by Sean Nash on 28 January 2025, 13:08 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21217 CD1/CD2/CD3/CD4 should be refactored? Reply by Vinayakam Mani on 28 January 2025, 16:14 > Done Revision Comment by Sean Nash on 28 January 2025, 13:09 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21218 Does floater have a component code? Or is this for all floaters? Reply by Vinayakam Mani on 28 January 2025, 16:24 > Done. Revision Comment by Sean Nash on 28 January 2025, 12:58 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21207 Primary heater = D5. Reply by Vinayakam Mani on 28 January 2025, 16:26 > Done. Revision Comment by Sean Nash on 28 January 2025, 13:00 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21208 BLD = D42. Reply by Vinayakam Mani on 28 January 2025, 16:27 > Done. Revision Comment by Sean Nash on 28 January 2025, 13:01 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21209 BLD = D42. Reply by Vinayakam Mani on 28 January 2025, 16:29 > Done. Revision Comment by Sean Nash on 28 January 2025, 13:03 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21210 Should we have D19..D26 here? Reply by Vinayakam Mani on 28 January 2025, 16:32 > Done Revision Comment by Sean Nash on 28 January 2025, 13:03 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21211 Are these obsolete now? Reply by Vinayakam Mani on 28 January 2025, 16:33 > Yeap. will remove once we identify the UF pumps. Revision Comment by Sean Nash on 28 January 2025, 13:04 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21212 Are these bits TBD or reserved? Reply by Vinayakam Mani on 28 January 2025, 16:35 > Reserved. Revision Comment by Sean Nash on 28 January 2025, 13:05 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21213 Should TD# be changed to new component code? Reply by Vinayakam Mani on 28 January 2025, 16:41 > Done. Revision Comment by Sean Nash on 28 January 2025, 13:06 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21214 D5? Reply by Vinayakam Mani on 28 January 2025, 16:48 > Done. Revision Comment by Sean Nash on 28 January 2025, 13:07 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21215 Does floater 2 have a component code? Reply by Vinayakam Mani on 28 January 2025, 16:50 > This is unused for now. Initially we assumed floater has two > outputs (floater 1 and floater 2) and created these routines. > may be removed once FPGA cleanup is done. ---------------------------------------- File: firmware/App/Services/FpgaDD.h Revision Comment by Sean Nash on 28 January 2025, 12:57 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21206 Do these heaters have a component code? I think they do. Why haven't these functions been refactored? Reply by Vinayakam Mani on 28 January 2025, 16:53 > Done. ---------------------------------------- File: firmware/App/Services/Messaging.c Revision Comment by Sean Nash on 28 January 2025, 11:07 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21169 This is problematic. New msg IDs beyond this point in list will have 2 possible ID numbers. I think these need to be at the end of the normal message section of the list and given a very high ID number (e.g. 0x7000) so there is no chance of conflicting with other message IDs. Reply by Vinayakam Mani on 28 January 2025, 17:01 > Done. ---------------------------------------- File: firmware/App/Services/TDInterface.h Revision Comment by Sean Nash on 28 January 2025, 13:10 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21219 Should be DIALYSATE flow rate. Should min/max rate be moved to dialysate pumps header file? Reply by Vinayakam Mani on 28 January 2025, 17:10 > Done. ---------------------------------------- File: firmware/source/sys_main.c Revision Comment by Sean Nash on 28 January 2025, 13:41 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21222 Should only be if PUMP_TEST defined? Reply by Vinayakam Mani on 29 January 2025, 17:46 > Done ---------------------------------------- File: firmware/App/Controllers/PistonPumpControl.c Revision Comment by Sean Nash on 27 January 2025, 16:45 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21157 Should this be at the top? Any reason to include a bunch of header files if there's no code below? Reply by Vinayakam Mani on 28 January 2025, 11:43 > Bringing it top of header file, seems compiler not finding > the #define and that makes code disabled even though its > declared. Revision Comment by Sean Nash on 27 January 2025, 16:47 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21158 Group is inside #if/endif block so this end group marker should be above the #endif. Reply by Vinayakam Mani on 28 January 2025, 11:44 > Done. ---------------------------------------- File: firmware/App/Controllers/PistonPumpControl.h Revision Comment by Sean Nash on 27 January 2025, 16:45 https://devapps.diality.us/cru/DD-LEAH-1971-1#c21156 Should this be at the very top of this header file? Reply by Vinayakam Mani on 28 January 2025, 13:41 > Done. --- ID: DD-LEAH-1971-1 https://devapps.diality.us/cru/DD-LEAH-1971-1 Title: DD-LEAH-1971_DD Integration Changes Statement of Objectives: State: Closed Summary: Author: Vinayakam Mani Moderator: Vinayakam Mani Reviewers: (2 active, 2 completed*) Sean Nash (*) Michael Garthwaite (*) jpaguio Dara Navaei