This is a list of all comments for HD-DEN-12215-2. Review Summary: No summary ---------------------------------------- File: firmware/App/Controllers/Switches.c Revision Comment by Sean Nash on 08 March 2022, 15:58 https://devapps.diality.us/cru/HD-DEN-12215-2#c12418 I don't think this is the right place to clear alarm conditions. This area of code is updating the actual detected switch states. But switch states are overridable so we should consider any overrides when clearing alarm conditions. Reply by Darren Cox on 10 March 2022, 10:14 > Moved and used getSwitchStatus to ensure overrides work. Reply by Sean Nash on 22 March 2022, 14:22 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModePreTreat.c Revision Comment by Michael Garthwaite on 08 March 2022, 16:33 https://devapps.diality.us/cru/HD-DEN-12215-2#c12427 Similiar to how the check is done in ModePostTreat.c, I think we should check if the heparin parameters are > 0.0 than NEARLY_ZERO. Reply by Darren Cox on 09 March 2022, 14:47 > Sean said leave it as is. No harm to be done. Reply by Michael Garthwaite on 22 March 2022, 14:04 > RESOLVED IN CODE WALKTHROUGH. Revision Comment by Sean Nash on 08 March 2022, 16:21 https://devapps.diality.us/cru/HD-DEN-12215-2#c12420 I think you have these switch checks in the right pre-treatment sub-modes, but I think each of these sub-modes will have a "stopped" state in its state machine where it will go if an alarm occurs and in that state the user should be allowed to open the door to possibly address the issue. So I think we need to figure out how to exempt those alarm states. Reply by Darren Cox on 10 March 2022, 10:18 > Added exception for Stopped sub states and before door should > be closed. Reply by Sean Nash on 22 March 2022, 14:21 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModePostTreat.c Revision Comment by Michael Garthwaite on 08 March 2022, 16:32 https://devapps.diality.us/cru/HD-DEN-12215-2#c12426 Out of scope of the changes here but hepRate should be getting TREATMENT_PARAM_HEPARIN_DISPENSE_RATE instead of TREATMENT_PARAM_HEPARIN_BOLUS_VOLUME. Reply by Darren Cox on 10 March 2022, 10:15 > Good catch. Updated. Reply by Michael Garthwaite on 22 March 2022, 14:04 > RESOLVED IN CODE WALKTHROUGH. Revision Comment by Sean Nash on 08 March 2022, 16:24 https://devapps.diality.us/cru/HD-DEN-12215-2#c12421 Do we want to check pump track switch too? Reply by Darren Cox on 10 March 2022, 10:16 > Added Reply by Sean Nash on 22 March 2022, 14:18 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 08 March 2022, 16:25 https://devapps.diality.us/cru/HD-DEN-12215-2#c12422 Why does door need to be closed for this state? Reply by Sean Nash on 22 March 2022, 14:46 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/SelfTests.c Revision Comment by Sean Nash on 08 March 2022, 16:28 https://devapps.diality.us/cru/HD-DEN-12215-2#c12423 Should we check pump track switch too? Reply by Darren Cox on 10 March 2022, 10:16 > Added. Reply by Darren Cox on 10 March 2022, 10:16 > Added. Reply by Sean Nash on 22 March 2022, 14:15 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 08 March 2022, 16:28 https://devapps.diality.us/cru/HD-DEN-12215-2#c12424 Should we check pump track switch too? Reply by Sean Nash on 22 March 2022, 14:13 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/source/sys_link.cmd Revision Comment by Sean Nash on 08 March 2022, 16:05 https://devapps.diality.us/cru/HD-DEN-12215-2#c12419 If I understand what you did here, I think we're good updating from HALCoGen w/o breaking the build but if we change stack size(s) we will need to manually update the #if 1 section accordingly after HALCoGen updates this file. Reply by Darren Cox on 09 March 2022, 11:26 > Yes. It writes the entire block for MEMORY and SECTIONS so > the entire section must be overridden rather than single > items. Reply by Sean Nash on 22 March 2022, 14:12 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: MsgDefs.h Revision Comment by Sean Nash on 11 March 2022, 10:36 https://devapps.diality.us/cru/HD-DEN-12215-2#c12519 Where is 0xA04E? Reply by Michael Garthwaite on 11 March 2022, 11:09 > 0xA04E was originally > MSG_ID_DG_HD_COMMUNICATION_STATUS_OVERRIDE. It was moved to > 0xA008 due to an out of sync error in our lovely excel sheet. > MSG_ID_DG_SET_FANS_RPM_ALARM_START_TIME_OFFSET should now be > 0xA04E. > > Note: These values are what is currently reflected on > staging. Reply by Sean Nash on 22 March 2022, 14:10 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ConsumableSelfTest.c Revision Comment by Sean Nash on 17 March 2022, 13:28 https://devapps.diality.us/cru/HD-DEN-12215-2#c12523 What is the reason for these changes? Just reading the code, it makes more sense to me that way it was. Reply by Darren Cox on 22 March 2022, 10:42 > This was part of the merge from staging branch. Reply by Sean Nash on 22 March 2022, 13:14 > It looks like staging branch has things the way it was too. Reply by Sean Nash on 22 March 2022, 14:19 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: NVDataMgmtDGRecords.h Revision Comment by Sean Nash on 22 March 2022, 13:04 https://devapps.diality.us/cru/HD-DEN-12215-2#c12583 Should be since last service (not last treatment). Reply by Dara Navaei on 22 March 2022, 14:00 > Done. Reply by Sean Nash on 22 March 2022, 14:07 > RESOLVED in CODE WALKTHROUGH. --- ID: HD-DEN-12215-2 https://devapps.diality.us/cru/HD-DEN-12215-2 Title: HD-DEN-12215_SW Dev Sprint 65 Darren Statement of Objectives: State: Closed Summary: Author: Darren Cox Moderator: Darren Cox Reviewers: (1 active, 3 completed*) Sean Nash (*) Michael Garthwaite (*) Dara Navaei (*) hnguyen