This is a list of all comments for DD-LEAH-236-1. Review Summary: No summary ---------------------------------------- File: firmware/App/Controllers/ConcentratePumps.c Revision Comment by Sean Nash on 08 November 2024, 09:24 https://devapps.diality.us/cru/DD-LEAH-236-1#c20836 How would Dialin user request to stop the pump? Reply by Vinayakam Mani on 08 November 2024, 12:16 > testConcentratePumpParkRequestOverride - this function helps > to stop the pump (if it is running) and park the pumps (based > on the request). Reply by Sean Nash on 10 November 2024, 11:20 > That function allows you to stop and park the pump but I > don't think it allows you to stop and not park the pump. Reply by Vinayakam Mani on 11 November 2024, 17:06 > Done. ---------------------------------------- File: firmware/App/Controllers/DialysatePumps.h Revision Comment by Sean Nash on 08 November 2024, 09:23 https://devapps.diality.us/cru/DD-LEAH-236-1#c20835 Can we handle start/stop in a single Dialin message? Reply by Vinayakam Mani on 11 November 2024, 17:07 > Done ---------------------------------------- File: firmware/App/Controllers/Heaters.h Revision Comment by Sean Nash on 08 November 2024, 09:29 https://devapps.diality.us/cru/DD-LEAH-236-1#c20837 Can start/stop be handled in single Dialin message? Reply by Vinayakam Mani on 11 November 2024, 17:07 > Done. ---------------------------------------- File: firmware/App/Controllers/Valves.c Revision Comment by Sean Nash on 14 November 2024, 08:55 https://devapps.diality.us/cru/DD-LEAH-236-1#c20910 "Valve" should be plural in these function names. Change "the all" to "all". Reply by Vinayakam Mani on 14 November 2024, 15:39 > Done. ---------------------------------------- File: firmware/App/Modes/BalancingChamber.c Revision Comment by Sean Nash on 08 November 2024, 10:03 https://devapps.diality.us/cru/DD-LEAH-236-1#c20842 Add "F" suffix to floating point literals. Reply by Vinayakam Mani on 11 November 2024, 17:07 > Done Revision Comment by Sean Nash on 08 November 2024, 10:14 https://devapps.diality.us/cru/DD-LEAH-236-1#c20845 These variables are initialized in init function. No need to initialize here. Reply by Vinayakam Mani on 11 November 2024, 17:07 > Done. Revision Comment by Sean Nash on 08 November 2024, 10:07 https://devapps.diality.us/cru/DD-LEAH-236-1#c20843 What are units for this frequency? Looks like switches per minute? Does anybody need this? Reply by Vinayakam Mani on 11 November 2024, 17:08 > Yes, Switches per minute. Just provided incase somebody need > to override the switching rate for testing purpose. Revision Comment by Sean Nash on 08 November 2024, 10:23 https://devapps.diality.us/cru/DD-LEAH-236-1#c20848 Is this initial increment needed? The exec function increments this counter. Reply by Vinayakam Mani on 11 November 2024, 17:10 > Done. Revision Comment by Sean Nash on 08 November 2024, 10:43 https://devapps.diality.us/cru/DD-LEAH-236-1#c20852 Fix indent. Reply by Vinayakam Mani on 11 November 2024, 17:10 > Done. Revision Comment by Sean Nash on 08 November 2024, 10:27 https://devapps.diality.us/cru/DD-LEAH-236-1#c20849 Need to handle situation where switching time arrives and pressure has not yet indicated fill completed (alarm/fault). Reply by Vinayakam Mani on 11 November 2024, 17:11 > Will handle in next revision. created a placeholder. Revision Comment by Sean Nash on 08 November 2024, 10:40 https://devapps.diality.us/cru/DD-LEAH-236-1#c20850 Fix indent. Reply by Vinayakam Mani on 11 November 2024, 17:11 > Done. Revision Comment by Sean Nash on 08 November 2024, 10:42 https://devapps.diality.us/cru/DD-LEAH-236-1#c20851 Fix indents. Reply by Vinayakam Mani on 11 November 2024, 17:11 > Done. Revision Comment by Sean Nash on 08 November 2024, 10:46 https://devapps.diality.us/cru/DD-LEAH-236-1#c20854 Need to handle situation where switch time arrives and pressure does not yet indicate fill completed. Reply by Vinayakam Mani on 11 November 2024, 17:11 > Will handle in next revision. created a placeholder. Revision Comment by Sean Nash on 08 November 2024, 10:45 https://devapps.diality.us/cru/DD-LEAH-236-1#c20853 I think this if .. else if .. would be cleaner as a nested if. Reply by Vinayakam Mani on 11 November 2024, 17:12 > Updated. Revision Comment by Sean Nash on 08 November 2024, 11:02 https://devapps.diality.us/cru/DD-LEAH-236-1#c20856 It looks like this check is only done at beginning of dialysate delivery and, once everything looks ok, we end bypass and never check again. I think we need to always be checking temp, conductivity, and pressure and bypass whenever anything is off. Reply by Vinayakam Mani on 11 November 2024, 17:12 > Done. removed the state and added Execmonitor to check these > parameters. Revision Comment by Sean Nash on 08 November 2024, 10:50 https://devapps.diality.us/cru/DD-LEAH-236-1#c20855 This state, when used, appears to delay start of next fill cycle by 50ms. Reply by Vinayakam Mani on 11 November 2024, 17:14 > Done. Revision Comment by Sean Nash on 08 November 2024, 11:06 https://devapps.diality.us/cru/DD-LEAH-236-1#c20857 When are you planning on implementing this? Do we need for demo? Reply by Vinayakam Mani on 11 November 2024, 17:14 > Will handle in next revision. Not sure alarm needs to be > implemented for 6th demo. Revision Comment by Sean Nash on 08 November 2024, 11:08 https://devapps.diality.us/cru/DD-LEAH-236-1#c20858 Do we need to include whether or not dialyzer is bypassed? I think TD needs to know if you're bypassing due to invalid temp/cond/pres. Is 1 second broadcast interval fast enough? Reply by Vinayakam Mani on 11 November 2024, 17:15 > was already taken care., There are two flags being published > ( isDialysateGoodtoDeliver , isDialDelInProgress ). ---------------------------------------- File: firmware/App/Modes/BalancingChamber.h Revision Comment by Sean Nash on 08 November 2024, 09:36 https://devapps.diality.us/cru/DD-LEAH-236-1#c20839 The "is" fields should be BOOL I think. Reply by Vinayakam Mani on 11 November 2024, 17:17 > Done. ---------------------------------------- File: firmware/App/Modes/ModeGenDialysate.c Revision Comment by Sean Nash on 14 November 2024, 09:00 https://devapps.diality.us/cru/DD-LEAH-236-1#c20911 Fix "Dialysatey" (remove y). And I think "Gend" should be GenD" in all of these function names. Reply by Vinayakam Mani on 14 November 2024, 15:39 > Done. Revision Comment by Sean Nash on 14 November 2024, 09:00 https://devapps.diality.us/cru/DD-LEAH-236-1#c20912 Remove extra blank line. Reply by Vinayakam Mani on 14 November 2024, 15:38 > Done. Revision Comment by Sean Nash on 14 November 2024, 09:01 https://devapps.diality.us/cru/DD-LEAH-236-1#c20914 If no inputs or outpus, say "none". Reply by Vinayakam Mani on 14 November 2024, 15:38 > Done. Revision Comment by Sean Nash on 14 November 2024, 09:01 https://devapps.diality.us/cru/DD-LEAH-236-1#c20913 If no alarms triggered, don't include an Alarm details line in the header. Reply by Vinayakam Mani on 14 November 2024, 15:38 > Done. Revision Comment by Sean Nash on 13 November 2024, 11:02 https://devapps.diality.us/cru/DD-LEAH-236-1#c20901 Seems like this would be a separate state or maybe we go to pause state too. Reply by Vinayakam Mani on 13 November 2024, 17:47 > Removed balancing chamber pause state. Revision Comment by Sean Nash on 14 November 2024, 09:06 https://devapps.diality.us/cru/DD-LEAH-236-1#c20915 Who is calling this function? If this function is not doing anything other than changing modes, do we really need this function? Caller can just call requestNewOperationMode(). Reply by Vinayakam Mani on 14 November 2024, 15:38 > Done. ---------------------------------------- File: firmware/App/Modes/ModeStandby.c Revision Comment by Sean Nash on 13 November 2024, 09:00 https://devapps.diality.us/cru/DD-LEAH-236-1#c20898 Seems strange that this function is in this unit. We would not be in standby mode when this request is made. Would be more appropriate to move this function to ModeGenDialysate. Reply by Vinayakam Mani on 13 November 2024, 17:48 > Done. ---------------------------------------- File: firmware/App/Modes/OperationModes.c Revision Comment by Sean Nash on 08 November 2024, 11:16 https://devapps.diality.us/cru/DD-LEAH-236-1#c20859 Remove extra blank line. Reply by Vinayakam Mani on 11 November 2024, 17:19 > Done. Revision Comment by Sean Nash on 08 November 2024, 11:17 https://devapps.diality.us/cru/DD-LEAH-236-1#c20860 Do we need to initialize standby and dialysate delivery modes? Reply by Vinayakam Mani on 11 November 2024, 17:19 > Done. Revision Comment by Sean Nash on 08 November 2024, 11:20 https://devapps.diality.us/cru/DD-LEAH-236-1#c20861 sizeof() for an enum will probably come back as 1 or 2 bytes depending on size of enum list. I suspect Dialin is sending a U32 (4 bytes) here. Reply by Vinayakam Mani on 11 November 2024, 17:19 > Done. ---------------------------------------- File: firmware/App/Services/Messaging.c Revision Comment by Sean Nash on 13 November 2024, 09:03 https://devapps.diality.us/cru/DD-LEAH-236-1#c20900 Should this be "if ( DD_MODE_GEND == ddMode )" ? I don't think this message makes sense in any other mode. Reply by Vinayakam Mani on 13 November 2024, 17:48 > Done. ---------------------------------------- File: firmware/App/Services/TDInterface.c Revision Comment by Sean Nash on 14 November 2024, 09:12 https://devapps.diality.us/cru/DD-LEAH-236-1#c20917 ModeGenDialysate should move up before ModeInitPOST to be in alphabetical order. Reply by Vinayakam Mani on 14 November 2024, 15:37 > Done. Revision Comment by Sean Nash on 08 November 2024, 09:45 https://devapps.diality.us/cru/DD-LEAH-236-1#c20840 Should have other variables from dialysate delivery command msg from TD. i.e. UF rate, target dialysate temp, dialyzer bypass, and concentrate types. Reply by Vinayakam Mani on 12 November 2024, 17:35 > Done. Revision Comment by Sean Nash on 14 November 2024, 09:13 https://devapps.diality.us/cru/DD-LEAH-236-1#c20918 Change "&&" to "||". Also, I think we need another condition added here because I don't think we should be setting all of these values if things above aren't right. Reply by Vinayakam Mani on 14 November 2024, 15:36 > Done. Revision Comment by Sean Nash on 14 November 2024, 09:15 https://devapps.diality.us/cru/DD-LEAH-236-1#c20919 I don't think the "|=" is accomplishing anything useful here - this is effectively setting result to TRUE in all cases. And I don't think we should be setting result to TRUE here necessarily. If we set result to FALSE above, it should stay FALSE. Reply by Vinayakam Mani on 14 November 2024, 15:36 > Done. ---------------------------------------- File: firmware/App/Services/TDInterface.h Revision Comment by Sean Nash on 08 November 2024, 09:47 https://devapps.diality.us/cru/DD-LEAH-236-1#c20841 Need TD dialysate deliver message handler to set params. Reply by Vinayakam Mani on 12 November 2024, 17:36 > Done. Revision Comment by Sean Nash on 14 November 2024, 09:11 https://devapps.diality.us/cru/DD-LEAH-236-1#c20916 Indent function name more to align like others. Reply by Vinayakam Mani on 14 November 2024, 15:36 > Done. ---------------------------------------- File: firmware/App/Services/Messaging.h Revision Comment by Sean Nash on 13 November 2024, 09:02 https://devapps.diality.us/cru/DD-LEAH-236-1#c20899 This message handler would be more appropriately placed in TDInterface unit. Reply by Vinayakam Mani on 13 November 2024, 17:48 > Done. --- ID: DD-LEAH-236-1 https://devapps.diality.us/cru/DD-LEAH-236-1 Title: DD-LEAH-236_FW DD Operation Modes Statement of Objectives: State: Closed Summary: Author: Vinayakam Mani Moderator: Vinayakam Mani Reviewers: (2 active, 2 completed*) Sean Nash (*) Dara Navaei (*) jpaguio Michael Garthwaite