This is a list of all comments for DG-DEN-14604-1. Review Summary: No summary ---------------------------------------- File: firmware/App/Controllers/Heaters.c Revision Comment by Sean Nash on 02 February 2023, 08:53 https://devapps.diality.us/cru/DG-DEN-14604-1#c16297 This is an input. Reply by Dara Navaei on 03 February 2023, 08:25 > Done Reply by Sean Nash on 03 February 2023, 09:12 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/Pressures.c Revision Comment by Sean Nash on 03 February 2023, 10:15 https://devapps.diality.us/cru/DG-DEN-14604-1#c16346 Shouldn't this one stay "MIN"? Reply by Dara Navaei on 03 February 2023, 10:20 > Done Reply by Sean Nash on 03 February 2023, 12:49 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModeHeatDisinfect.c Revision Comment by Sean Nash on 02 February 2023, 08:22 https://devapps.diality.us/cru/DG-DEN-14604-1#c16290 Alignment. Do we even need this. Will get reset on transition back to heat disinfect. Reply by Dara Navaei on 03 February 2023, 08:34 > Done. Reply by Sean Nash on 03 February 2023, 09:15 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 02 February 2023, 08:27 https://devapps.diality.us/cru/DG-DEN-14604-1#c16291 Not fault mode? Reply by Dara Navaei on 03 February 2023, 08:31 > Removed the code. Reply by Sean Nash on 03 February 2023, 09:14 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 02 February 2023, 08:27 https://devapps.diality.us/cru/DG-DEN-14604-1#c16292 Even in release build? Reply by Dara Navaei on 03 February 2023, 08:28 > Removed the code. Reply by Sean Nash on 03 February 2023, 10:12 > RESOLVED in CODE WALKTHROUGH. Revision Comment by wbracken on 02 February 2023, 17:43 https://devapps.diality.us/cru/DG-DEN-14604-1#c16314 Maybe #define? Reply by Dara Navaei on 03 February 2023, 08:23 > These values are temporary and only when we build a debug > code. So I prefer not to have a #define for them. Reply by Sean Nash on 03 February 2023, 09:11 > I think #defines are still appropriate. Since it's debug > code, they can be placed right above the function (inside > the #ifndef RELEASE) so they do not get used in release > s/w. Reply by Dara Navaei on 03 February 2023, 10:32 > Done Reply by wbracken on 03 February 2023, 11:12 > RESOLVED IN CODE WALKTHROUGH Revision Comment by Sean Nash on 02 February 2023, 08:28 https://devapps.diality.us/cru/DG-DEN-14604-1#c16293 For a blank case like this, we should add a comment making it clear no action is intended. Reply by Dara Navaei on 03 February 2023, 08:27 > Done Reply by Sean Nash on 03 February 2023, 09:13 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModeHeatDisinfect.h Revision Comment by wbracken on 24 January 2023, 14:39 https://devapps.diality.us/cru/DG-DEN-14604-1#c16222 Comments Reply by Dara Navaei on 03 February 2023, 08:36 > Done. Reply by wbracken on 03 February 2023, 10:55 > RESOLVED IN CODE WALKTHROUGH ---------------------------------------- File: DGDefs.h Revision Comment by Sean Nash on 02 February 2023, 08:07 https://devapps.diality.us/cru/DG-DEN-14604-1#c16285 Is this temporary? Should it have #ifndef RELEASE? Reply by Dara Navaei on 03 February 2023, 08:36 > Removed the code. Reply by Sean Nash on 03 February 2023, 10:20 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 02 February 2023, 08:07 https://devapps.diality.us/cru/DG-DEN-14604-1#c16286 Temporary? Reply by Dara Navaei on 03 February 2023, 08:35 > Removed the code. Reply by Sean Nash on 03 February 2023, 10:19 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 03 February 2023, 09:19 https://devapps.diality.us/cru/DG-DEN-14604-1#c16334 Will this get picked up by doxygen? Do we want that? If not, maybe put this structure outside of the heat disinfect grouping. Reply by Dara Navaei on 03 February 2023, 10:07 > Done Reply by Sean Nash on 03 February 2023, 10:19 > The enum comments are now non-doxygen style, but the enum > is still in the group. Move it below the /**@}*/ to get it > out of HeatersState group. Reply by Dara Navaei on 03 February 2023, 10:35 > Done Reply by Sean Nash on 03 February 2023, 10:49 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: AlarmDefs.h Revision Comment by wbracken on 31 January 2023, 11:14 https://devapps.diality.us/cru/DG-DEN-14604-1#c16258 Need to verify alarms with SRS after merge with staging. Reply by Dara Navaei on 18 October 2023, 20:46 > RESOLVED IN CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Controllers/UVReactors.c Revision Comment by Sean Nash on 03 February 2023, 09:22 https://devapps.diality.us/cru/DG-DEN-14604-1#c16335 Shouldn't this just be an if (not an else if)? If both are unhealthy, we should activate both alarms, right? Reply by Dara Navaei on 03 February 2023, 10:01 > Done Reply by Sean Nash on 03 February 2023, 10:10 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModeChemicalDisinfect.c Revision Comment by Sean Nash on 02 February 2023, 08:17 https://devapps.diality.us/cru/DG-DEN-14604-1#c16288 Is this necessary? We're faulting. Won't be coming back. Even if we did come back, wouldn't init set this? Reply by Dara Navaei on 03 February 2023, 09:46 > No this has been removed. Reply by Sean Nash on 03 February 2023, 10:12 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 02 February 2023, 08:17 https://devapps.diality.us/cru/DG-DEN-14604-1#c16289 Even in release build? Reply by Dara Navaei on 03 February 2023, 08:35 > Let's keep this broadcast until this project is finished. Reply by Sean Nash on 03 February 2023, 10:12 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/SystemCommMessages.c Revision Comment by Sean Nash on 02 February 2023, 08:31 https://devapps.diality.us/cru/DG-DEN-14604-1#c16294 Declarations at top of scope. Reply by Dara Navaei on 03 February 2023, 08:25 > Done. Reply by Sean Nash on 03 February 2023, 09:13 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 03 February 2023, 09:29 https://devapps.diality.us/cru/DG-DEN-14604-1#c16336 Missing break? Reply by Dara Navaei on 03 February 2023, 09:43 > Done. Reply by Sean Nash on 03 February 2023, 10:10 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModeChemicalDisinfect.h Revision Comment by Sean Nash on 02 February 2023, 09:01 https://devapps.diality.us/cru/DG-DEN-14604-1#c16298 #ifndef RELEASE. Reply by Dara Navaei on 03 February 2023, 08:24 > This include is no longer needed. Reply by Sean Nash on 03 February 2023, 09:12 > RESOLVED in CODE WALKTHROUGH. --- ID: DG-DEN-14604-1 https://devapps.diality.us/cru/DG-DEN-14604-1 Title: DG-DEN-14604_DG HD Dev HD DG Dvt Update Part 9 Statement of Objectives: State: Closed Summary: Author: Dara Navaei Moderator: Dara Navaei Reviewers: (4 active, 2 completed*) Sean Nash (*) wbracken (*) Michael Garthwaite Darren Cox jtaylor Steve Jarpe