This is a list of all comments for DG-DEN-14646-1. Review Summary: No summary ---------------------------------------- File: firmware/App/Controllers/Heaters.c Revision Comment by Darren Cox on 27 February 2023, 15:09 https://devapps.diality.us/cru/DG-DEN-14646-1#c16546 Remove old function call. Reply by Darren Cox on 14 March 2023, 11:45 > RESOLVED IN CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Controllers/ConductivitySensors.c Revision Comment by Sean Nash on 27 February 2023, 09:56 https://devapps.diality.us/cru/DG-DEN-14646-1#c16514 Remove "endif" at end of comment? Reply by wbracken on 22 March 2023, 14:25 > Corrected Reply by Sean Nash on 27 March 2023, 09:36 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Darren Cox on 31 March 2023, 12:59 https://devapps.diality.us/cru/DG-DEN-14646-1#c17026 Does TODO need to be resolved? Reply by wbracken on 31 March 2023, 13:42 > Corrected Reply by Darren Cox on 31 March 2023, 13:45 > RESOLVED IN CODE WALKTHROUGH. Revision Comment by Sean Nash on 13 March 2023, 08:54 https://devapps.diality.us/cru/DG-DEN-14646-1#c16690 Use #define for bit mask. Remove extra ";". Reply by wbracken on 22 March 2023, 14:20 > Corrected Reply by Sean Nash on 27 March 2023, 09:35 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 13 March 2023, 08:55 https://devapps.diality.us/cru/DG-DEN-14646-1#c16691 Is it ok to ignore P if package has already been started? Should this ever happen? Reply by wbracken on 14 March 2023, 10:11 > Removed. Reply by Sean Nash on 20 March 2023, 09:09 > RESOLVED in CODE WALKTHROUGH. Revision Comment by jtaylor on 31 March 2023, 12:59 https://devapps.diality.us/cru/DG-DEN-14646-1#c17025 Missing space. Reply by wbracken on 31 March 2023, 13:42 > Corrected Reply by jtaylor on 31 March 2023, 13:43 > RESOLVED IN CODE WALKTHROUGH. Revision Comment by Sean Nash on 29 March 2023, 14:51 https://devapps.diality.us/cru/DG-DEN-14646-1#c16982 Spaces inside (). And explicit condition (i.e. ( TRUE == isConvNotValid ). Reply by Sean Nash on 30 March 2023, 19:04 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 13 March 2023, 09:02 https://devapps.diality.us/cru/DG-DEN-14646-1#c16692 Is prefix guaranteed to not be zero? Reply by wbracken on 14 March 2023, 10:07 > Yes. Reply by Sean Nash on 20 March 2023, 09:09 > RESOLVED in CODE WALKTHROUGH. Reply by jtaylor on 31 March 2023, 13:44 > RESOLVED IN CODE WALKTHROUGH. Revision Comment by jtaylor on 31 March 2023, 13:01 https://devapps.diality.us/cru/DG-DEN-14646-1#c17027 Extra space before the last paren. Reply by wbracken on 31 March 2023, 13:42 > Corrected Reply by Dara Navaei on 18 October 2023, 20:44 > RESOLVED IN CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Controllers/TemperatureSensors.c Revision Comment by Sean Nash on 13 March 2023, 09:04 https://devapps.diality.us/cru/DG-DEN-14646-1#c16693 Why is this indented? Reply by wbracken on 14 March 2023, 13:47 > Corrected. Reply by Sean Nash on 20 March 2023, 09:12 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/FPGA.c Revision Comment by Sean Nash on 13 March 2023, 09:09 https://devapps.diality.us/cru/DG-DEN-14646-1#c16695 Maybe we should put a bread crumb comment here to make it easier to find lines of code that need to be commented out when debugging (e.g. // comment this line for DEBUG). Reply by wbracken on 14 March 2023, 10:00 > Per our alarms discussion it was decided that this alarm was > no longer necessary. Reply by Sean Nash on 20 March 2023, 09:14 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 13 March 2023, 09:12 https://devapps.diality.us/cru/DG-DEN-14646-1#c16696 DEBUG bread crumb comment. Reply by Sean Nash on 20 March 2023, 09:15 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: Utilities.c Revision Comment by Darren Cox on 27 February 2023, 15:11 https://devapps.diality.us/cru/DG-DEN-14646-1#c16547 Default case sets status = FALSE. Setting to TRUE here is inconsistent with that. Reply by wbracken on 14 March 2023, 11:46 > Correct as is per conversation with Darren. Reply by Darren Cox on 14 March 2023, 12:21 > RESOLVED IN CODE WALKTHROUGH. Revision Comment by jtaylor on 27 February 2023, 08:48 https://devapps.diality.us/cru/DG-DEN-14646-1#c16512 extra CR/LF deleted; two statements on one line. Reply by wbracken on 14 March 2023, 11:46 > Corrected. Reply by jtaylor on 16 March 2023, 09:10 > RESOLVED IN CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/Voltages.h Revision Comment by Sean Nash on 13 March 2023, 09:06 https://devapps.diality.us/cru/DG-DEN-14646-1#c16694 A small thing, but 24V Main should come before first voltage. Reply by Sean Nash on 20 March 2023, 09:13 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/WatchdogMgmt.c Revision Comment by Sean Nash on 13 March 2023, 09:13 https://devapps.diality.us/cru/DG-DEN-14646-1#c16697 DEBUG bread crumb comment? Reply by wbracken on 22 March 2023, 14:15 > Added DEBUG WARNING Reply by Sean Nash on 27 March 2023, 09:35 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/Valves.c Revision Comment by Sean Nash on 20 March 2023, 09:13 https://devapps.diality.us/cru/DG-DEN-14646-1#c16877 Comment (and name) do not seem to fit anymore. No longer number of times or a count. Now a max time. Reply by wbracken on 22 March 2023, 14:21 > Alarm is based on a counter. The #define corresponds to a > 100ms time. Reply by Sean Nash on 27 March 2023, 09:34 > Yes, and #define has "COUNT" in its name which was my > point. Reply by wbracken on 27 March 2023, 10:29 > The variable that define is being compared to is > valveStateMismatchCounter which is why I called it such. Reply by Sean Nash on 27 March 2023, 10:43 > Can we rename variable to reflect it is a timer counter > at least? Reply by wbracken on 29 March 2023, 11:25 > Corrected Reply by Sean Nash on 30 March 2023, 11:35 > I don't see it, but resolving anyway. > RESOLVED in CODE WALKTHROUGH. --- ID: DG-DEN-14646-1 https://devapps.diality.us/cru/DG-DEN-14646-1 Title: DG-DEN-14646_Fpga Alarm Properties And Timing 3 Statement of Objectives: State: Closed Summary: Author: wbracken Moderator: wbracken Reviewers: (3 active, 3 completed*) Sean Nash (*) Darren Cox (*) jtaylor (*) Michael Garthwaite Dara Navaei Steve Jarpe