This is a list of all comments for DD-LDT-1873-1. Review Summary: No summary ---------------------------------------- File: firmware/App/Controllers/BalancingChamber.c Revision Comment by Sean Nash on 08 August 2025, 09:32 https://devapps.diality.us/cru/DD-LDT-1873-1#c23563 We are broadcasting BC data every 50ms? Why? Reply by Vinayakam Mani on 08 August 2025, 10:43 > Reverted. Revision Comment by Sean Nash on 18 August 2025, 07:50 https://devapps.diality.us/cru/DD-LDT-1873-1#c23790 isPressureDroppedDuringFill not initialized in init function. Reply by Vinayakam Mani on 18 August 2025, 09:58 > Done. Revision Comment by Sean Nash on 18 August 2025, 07:54 https://devapps.diality.us/cru/DD-LDT-1873-1#c23791 Remove extra blank line. Reply by Vinayakam Mani on 18 August 2025, 09:58 > Done. Revision Comment by Sean Nash on 12 August 2025, 13:53 https://devapps.diality.us/cru/DD-LDT-1873-1#c23651 Are we done with temporary code? Can we just remove it? Reply by Vinayakam Mani on 13 August 2025, 09:58 > Currently testing is ongoing. Will try to remove it end of > this sprint. ---------------------------------------- File: firmware/App/Controllers/Heaters.c Revision Comment by Sean Nash on 08 August 2025, 09:42 https://devapps.diality.us/cru/DD-LDT-1873-1#c23569 Per comment, change name to "adjustedPrimaryHeaterTargetTemp". Reply by Vinayakam Mani on 08 August 2025, 10:43 > Done. Revision Comment by Sean Nash on 08 August 2025, 09:35 https://devapps.diality.us/cru/DD-LDT-1873-1#c23565 If this function only supports D5 heater, why do we ask for a heater param? Why not refactor function to "void signalToResetPrimaryHeaterAdjustedTargetTemp( void )"? Reply by Vinayakam Mani on 08 August 2025, 10:44 > Done. Revision Comment by Sean Nash on 08 August 2025, 09:38 https://devapps.diality.us/cru/DD-LDT-1873-1#c23566 This function infers (in its name) that it is for the primary heater only. So why take a heater ID param? Reply by Vinayakam Mani on 08 August 2025, 10:44 > Done. Revision Comment by Sean Nash on 08 August 2025, 09:39 https://devapps.diality.us/cru/DD-LDT-1873-1#c23567 The "heater" parameter might not be the primary heater. Reply by Vinayakam Mani on 08 August 2025, 10:44 > Done. Revision Comment by Sean Nash on 08 August 2025, 09:40 https://devapps.diality.us/cru/DD-LDT-1873-1#c23568 Declarations should come first in a {} scope. And no need to align these "=". Reply by Vinayakam Mani on 08 August 2025, 10:44 > Done. Revision Comment by Sean Nash on 08 August 2025, 09:43 https://devapps.diality.us/cru/DD-LDT-1873-1#c23570 adjustedTargetTemp says it is for the primary heater (d5). Why are we saying it is for the trimmer heater here? Reply by Vinayakam Mani on 08 August 2025, 10:45 > these changes made for testing purposes. reverted to > original. ---------------------------------------- File: firmware/App/Controllers/Valves.c Revision Comment by Sean Nash on 08 August 2025, 09:44 https://devapps.diality.us/cru/DD-LDT-1873-1#c23571 Restore after testing. Reply by Vinayakam Mani on 08 August 2025, 10:46 > Reverted. ---------------------------------------- File: firmware/App/Modes/ModeGenDialysate.c Revision Comment by Sean Nash on 12 August 2025, 13:57 https://devapps.diality.us/cru/DD-LDT-1873-1#c23655 Align comment. Reply by Vinayakam Mani on 13 August 2025, 09:58 > Done. ---------------------------------------- File: firmware/App/Monitors/Pressure.c Revision Comment by Sean Nash on 08 August 2025, 09:46 https://devapps.diality.us/cru/DD-LDT-1873-1#c23572 Restore after testing. This kind of thing is typically done with broadcast interval overrides from Dialin (not temporary hard-coded changes). Reply by Vinayakam Mani on 08 August 2025, 10:46 > Reverted. ---------------------------------------- File: firmware/App/Services/AlarmMgmtDD.c Revision Comment by Sean Nash on 08 August 2025, 09:48 https://devapps.diality.us/cru/DD-LDT-1873-1#c23573 Why? Reply by Vinayakam Mani on 08 August 2025, 10:46 > I remember seeing once some CRC error. To continue > development, disabled fault mode for a while. Reply by Sean Nash on 11 August 2025, 09:51 > If you get a chance, try to find out which message (ID) is > having the CRC error and where it's coming from (which > channel) and why it's failing CRC check (e.g. was a frame > missing?) ---------------------------------------- File: firmware/App/Services/ROInterface.h Revision Comment by Sean Nash on 08 August 2025, 09:50 https://devapps.diality.us/cru/DD-LDT-1873-1#c23574 Should we rename the unit to FPInterface? Reply by Vinayakam Mani on 08 August 2025, 10:48 > Done. ---------------------------------------- File: firmware/App/Services/SystemCommDD.c Revision Comment by Sean Nash on 11 August 2025, 09:55 https://devapps.diality.us/cru/DD-LDT-1873-1#c23596 Update comment too. Reply by Vinayakam Mani on 13 August 2025, 09:59 > Done. Revision Comment by Sean Nash on 11 August 2025, 09:55 https://devapps.diality.us/cru/DD-LDT-1873-1#c23597 Change function name (RO to FP)? Reply by Vinayakam Mani on 13 August 2025, 09:59 > Done. Revision Comment by Sean Nash on 08 August 2025, 09:50 https://devapps.diality.us/cru/DD-LDT-1873-1#c23575 fpCommunicationStatus? Reply by Vinayakam Mani on 08 August 2025, 10:48 > Done. Revision Comment by Sean Nash on 11 August 2025, 09:56 https://devapps.diality.us/cru/DD-LDT-1873-1#c23598 Change name (RO to FP)? Reply by Vinayakam Mani on 13 August 2025, 09:59 > Done. ---------------------------------------- File: firmware/App/Controllers/ConcentratePumps.c Revision Comment by Sean Nash on 08 August 2025, 09:33 https://devapps.diality.us/cru/DD-LDT-1873-1#c23564 These are temporary test intervals, right? Please revert when testing is completed. Reply by Vinayakam Mani on 08 August 2025, 10:43 > Reverted. Revision Comment by Sean Nash on 12 August 2025, 13:55 https://devapps.diality.us/cru/DD-LDT-1873-1#c23652 I think these can just be done once after else. Reply by Vinayakam Mani on 13 August 2025, 09:57 > Done. Revision Comment by Sean Nash on 12 August 2025, 13:56 https://devapps.diality.us/cru/DD-LDT-1873-1#c23654 Should we range check pumpId and trigger s/w fault if out of range? Reply by Vinayakam Mani on 13 August 2025, 09:57 > Done. --- ID: DD-LDT-1873-1 https://devapps.diality.us/cru/DD-LDT-1873-1 Title: DD-LDT-1873_Dialysate Flow Rate DD Statement of Objectives: State: Closed Summary: Author: Vinayakam Mani Moderator: Vinayakam Mani Reviewers: (3 active, 2 completed*) Sean Nash (*) Michael Garthwaite (*) jpaguio Dara Navaei Daniel Ho