This is a list of all comments for HD-DEN-14561-1. Review Summary: No summary ---------------------------------------- File: firmware/App/Modes/ModePreTreat.c Revision Comment by Sean Nash on 19 January 2023, 09:16 https://devapps.diality.us/cru/HD-DEN-14561-1#c16141 This should probably be 600 since we'll only be transferring 300 to R1 in wet self-tests. Reply by Darren Cox on 19 January 2023, 10:57 > Old transfer was 600mL, new is 300mL. Res 1 should end up > with 1500 so added 300 to baseline, Res 2 was previously > ending with 500. Do we want it to end R2 with 300? Reply by Sean Nash on 19 January 2023, 15:46 > So R2 fill was 1100 and you say it ended up at 500. Would > be 800 if keep same at 1100. If we change to 600, would > end around 300 as you say - I think that's right. Reply by Darren Cox on 20 January 2023, 10:36 > Updated to 600. Reply by Sean Nash on 20 January 2023, 11:39 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 31 January 2023, 09:04 https://devapps.diality.us/cru/HD-DEN-14561-1#c16248 Alignment of comment. Reply by Sean Nash on 02 February 2023, 17:10 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 19 January 2023, 09:17 https://devapps.diality.us/cru/HD-DEN-14561-1#c16142 We should initialize these with a loop iterating through each reservoir (in case a reservoir is added/removed at some point). Reply by jtaylor on 20 January 2023, 12:33 > Done. Reply by Sean Nash on 02 February 2023, 17:07 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Dara Navaei on 08 February 2023, 08:31 https://devapps.diality.us/cru/HD-DEN-14561-1#c16421 Why is this variable an int? It should be U32. Reply by Sean Nash on 08 February 2023, 11:02 > Fixed. Reply by Dara Navaei on 08 February 2023, 13:04 > RESOLVED in CODE WALKTHROUGH. Revision Comment by wbracken on 08 February 2023, 10:03 https://devapps.diality.us/cru/HD-DEN-14561-1#c16422 Should be reservoirStatus instead of reservoirFlags. Reply by Sean Nash on 08 February 2023, 10:59 > Fixed. Reply by wbracken on 08 February 2023, 13:01 > RESOLVED IN CODE WALKTHROUGH Revision Comment by wbracken on 08 February 2023, 10:03 https://devapps.diality.us/cru/HD-DEN-14561-1#c16423 ReservoirStatus Reply by Sean Nash on 08 February 2023, 10:58 > Fixed. Reply by wbracken on 08 February 2023, 13:00 > RESOLVED IN CODE WALKTHROUGH Revision Comment by wbracken on 08 February 2023, 10:04 https://devapps.diality.us/cru/HD-DEN-14561-1#c16424 reservoirStatus Reply by Sean Nash on 08 February 2023, 10:44 > Fixed. Reply by wbracken on 08 February 2023, 13:00 > RESOLVED IN CODE WALKTHROUGH Revision Comment by wbracken on 08 February 2023, 10:04 https://devapps.diality.us/cru/HD-DEN-14561-1#c16425 reservoirStatus Reply by Sean Nash on 08 February 2023, 10:43 > Fixed. Reply by wbracken on 08 February 2023, 13:00 > RESOLVED IN CODE WALKTHROUGH Revision Comment by Sean Nash on 31 January 2023, 13:26 https://devapps.diality.us/cru/HD-DEN-14561-1#c16270 We don't want to signal here if we don't have an air pump. Reply by jtaylor on 31 January 2023, 15:43 > Done. We will want to verify that we don't disable all of the > resume fill calls. Reply by Sean Nash on 07 February 2023, 15:41 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 19 January 2023, 09:19 https://devapps.diality.us/cru/HD-DEN-14561-1#c16143 Remove extra blank line. Reply by jtaylor on 20 January 2023, 15:34 > Done. Reply by Sean Nash on 02 February 2023, 17:08 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 19 January 2023, 09:20 https://devapps.diality.us/cru/HD-DEN-14561-1#c16144 This looks like we're skipping draining of reservoirs before doing any filling if priming disabled. I think we always want to drain both reservoirs first. Also, if we go to fill state like this, will it do a flush or a normal fill? I think it will do a flush fill since flushed flag is still FALSE. Reply by jtaylor on 20 January 2023, 12:20 > Done, initial drain of the reservoirs, prior to syncing with > the self-test. Reply by Sean Nash on 02 February 2023, 17:08 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 31 January 2023, 09:02 https://devapps.diality.us/cru/HD-DEN-14561-1#c16247 I don't see this being used anywhere. Remove. Reply by jtaylor on 07 February 2023, 15:25 > Done. Reply by Sean Nash on 07 February 2023, 16:21 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 31 January 2023, 13:30 https://devapps.diality.us/cru/HD-DEN-14561-1#c16271 What does flush complete have to do with draining here? Reply by jtaylor on 01 February 2023, 11:45 > Misalignment of terminology. I treated a reservoir "flush" as > a fill/drain cycle, ending with the reservoir empty. > "flushComplete" had been set accordingly. Test removed. Reply by Sean Nash on 02 February 2023, 17:15 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 31 January 2023, 12:55 https://devapps.diality.us/cru/HD-DEN-14561-1#c16263 Another way to handle making sure we get past s.t. dry state is to not set startNormalFill on res until op mode state machine gets to s.t. dry state. Then don't need to change this if to consider pre-tx state. Reply by jtaylor on 07 February 2023, 15:21 > This works, even if the s.t. dry test is skipped. Reply by Sean Nash on 07 February 2023, 16:24 > RESOLVED in CODE WALKTHROUGH. Revision Comment by wbracken on 20 January 2023, 14:29 https://devapps.diality.us/cru/HD-DEN-14561-1#c16194 Should align Reply by jtaylor on 20 January 2023, 15:35 > Done. Reply by wbracken on 25 January 2023, 16:45 > RESOLVED IN CODE WALKTHROUGH Revision Comment by Sean Nash on 31 January 2023, 13:05 https://devapps.diality.us/cru/HD-DEN-14561-1#c16265 Seems like this flush tracking logic should be done in fill complete state (after fill, not before). Reply by jtaylor on 31 January 2023, 15:42 > Done. Reply by Sean Nash on 02 February 2023, 17:12 > RESOLVED in CODE WALKTHROUGH. Revision Comment by wbracken on 01 February 2023, 16:14 https://devapps.diality.us/cru/HD-DEN-14561-1#c16281 Alignment Reply by jtaylor on 02 February 2023, 10:32 > Done. Reply by Sean Nash on 02 February 2023, 17:16 > RESOLVED in CODE WALKTHROUGH. Revision Comment by wbracken on 08 February 2023, 10:06 https://devapps.diality.us/cru/HD-DEN-14561-1#c16426 reservoirStatus Reply by Sean Nash on 08 February 2023, 10:38 > Fixed. Reply by wbracken on 08 February 2023, 13:00 > RESOLVED IN CODE WALKTHROUGH Revision Comment by Sean Nash on 31 January 2023, 09:15 https://devapps.diality.us/cru/HD-DEN-14561-1#c16249 Add spaces around (). And literal should go to right unless using == operator. Reply by jtaylor on 07 February 2023, 15:24 > Done. Reply by Sean Nash on 07 February 2023, 16:15 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 31 January 2023, 09:19 https://devapps.diality.us/cru/HD-DEN-14561-1#c16250 We are asking DG to set the currently active reservoir as the active reservoir? So no change? Reply by jtaylor on 07 February 2023, 15:21 > This turns off the trimmer hearter, to smooth heater control > (as before). The switch state (next up) actually makes the > reservoir switch, the next state. Reply by Sean Nash on 07 February 2023, 16:04 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 31 January 2023, 13:00 https://devapps.diality.us/cru/HD-DEN-14561-1#c16264 Add blank line between declaration and code. Reply by jtaylor on 31 January 2023, 13:14 > Done. Reply by Dara Navaei on 19 October 2023, 09:07 > RESOLVED in CODE WALKTHROUGH Revision Comment by wbracken on 08 February 2023, 10:11 https://devapps.diality.us/cru/HD-DEN-14561-1#c16427 reservoirStatus Reply by Sean Nash on 08 February 2023, 10:35 > Fixed. Reply by wbracken on 08 February 2023, 12:59 > RESOLVED IN CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Modes/SelfTests.c Revision Comment by Sean Nash on 02 February 2023, 17:01 https://devapps.diality.us/cru/HD-DEN-14561-1#c16304 This simplified equation works because we added up flow (mL/min) measurements for 1 minute. If we change displacement time, it falls apart (not generalized) which happens in second displacement below (now that transfer time was reduced to 30 sec). Let's generalize this math to clarify it and make it work regardless of time. Suggested approach: -integrate volume above properly (don't just sum rates over time). -Should be: fmdIntegratedVolume += ( getMeasuredDialInFlowRate() / (F32)( ( SEC_PER_MIN * MS_PER_SECOND ) / TASK_GENERAL_INTERVAL ) ); -Then we don't need to convert here at end of displacement - can just use fmdIntegratedVolume as is in verify state. Reply by jtaylor on 07 February 2023, 15:17 > Done (By Darren) Reply by Sean Nash on 07 February 2023, 15:40 > RESOLVED in CODE WALKTHROUGH. Revision Comment by wbracken on 01 February 2023, 16:44 https://devapps.diality.us/cru/HD-DEN-14561-1#c16282 #define? Reply by jtaylor on 02 February 2023, 10:30 > Done. Reply by wbracken on 03 February 2023, 10:59 > RESOLVED IN CODE WALKTHROUGH Revision Comment by Sean Nash on 02 February 2023, 17:04 https://devapps.diality.us/cru/HD-DEN-14561-1#c16305 This math no longer works because we changed the displacement time from 1 minute (which allowed for this simplified approach) to 30 seconds. Generalize the math. Suggested approach: -integrate volume above properly (don't just sum rates over time). -Should be: fmdIntegratedVolume += ( getMeasuredDialInFlowRate() / (F32)( ( SEC_PER_MIN * MS_PER_SECOND ) / TASK_GENERAL_INTERVAL ) ); -Then we don't need to convert here at end of displacement - can just use fmdIntegratedVolume as is in verify state. Reply by jtaylor on 07 February 2023, 15:11 > Done (by Darren) Reply by Sean Nash on 07 February 2023, 15:34 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 02 February 2023, 17:05 https://devapps.diality.us/cru/HD-DEN-14561-1#c16306 Do we understand why this is needed? Reply by jtaylor on 07 February 2023, 15:16 > The normal value is too tight for the current hardware; it > always fails this test, without the wider volume tolerance. Reply by Sean Nash on 07 February 2023, 15:39 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/Prime.c Revision Comment by Sean Nash on 19 January 2023, 09:27 https://devapps.diality.us/cru/HD-DEN-14561-1#c16147 There are two dialysate prime states. First goes through dialyzer and both DPs are running and we still want 300 rate. Second bypasses dialyzer and only DPi is running and we want 600 rate. Reply by Darren Cox on 19 January 2023, 10:55 > Updated for only reservoir 2 fill state. Reply by Sean Nash on 19 January 2023, 15:43 > RESOLVED in CODE WALKTHROUGH. Revision Comment by wbracken on 08 February 2023, 10:15 https://devapps.diality.us/cru/HD-DEN-14561-1#c16428 getReservoirFillStatus is a function Reply by Sean Nash on 08 February 2023, 10:34 > Fixed. Reply by wbracken on 08 February 2023, 12:59 > RESOLVED IN CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Controllers/SyringePump.c Revision Comment by Darren Cox on 17 January 2023, 14:22 https://devapps.diality.us/cru/HD-DEN-14561-1#c16070 This config check is unnecessary and should be removed. The vars should always be initialized. Enable/Disable Alarms are done when checking the direction, etc. Reply by jtaylor on 20 January 2023, 12:43 > Done. Reply by Darren Cox on 31 January 2023, 11:48 > RESOLVED IN CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Services/AlarmMgmt.c Revision Comment by Sean Nash on 04 February 2023, 18:34 https://devapps.diality.us/cru/HD-DEN-14561-1#c16366 This is not going to work. Revisit. See code review for DEN-14749 for correct arrangement. Reply by jtaylor on 07 February 2023, 15:14 > Done. Reply by Sean Nash on 07 February 2023, 15:34 > RESOLVED in CODE WALKTHROUGH. Revision Comment by wbracken on 19 January 2023, 09:02 https://devapps.diality.us/cru/HD-DEN-14561-1#c16140 Alignment Reply by jtaylor on 20 January 2023, 12:38 > Done. Reply by wbracken on 25 January 2023, 16:48 > RESOLVED IN CODEWALKTHROUGH ---------------------------------------- File: firmware/App/Modes/ModeTreatment.c Revision Comment by wbracken on 07 February 2023, 15:02 https://devapps.diality.us/cru/HD-DEN-14561-1#c16393 Add pendingUFVolumeChange, pressTreatmentTimeSecs, pendingParamChangesTimer, Reply by jtaylor on 07 February 2023, 15:26 > Done (By Darren). Reply by wbracken on 07 February 2023, 15:52 > RESOLVED IN CODE WALKTHROUGH --- ID: HD-DEN-14561-1 https://devapps.diality.us/cru/HD-DEN-14561-1 Title: HD-DEN-14561_Release Preprocessor Flag Specifies Code That Conflicts With SW Config Debug Statement of Objectives: State: Closed Summary: Author: jtaylor Moderator: jtaylor Reviewers: (3 active, 3 completed*) Sean Nash (*) wbracken (*) Dara Navaei (*) Michael Garthwaite Darren Cox Steve Jarpe