This is a list of all comments for DG-DEN-9480-1. Review Summary: No summary ---------------------------------------- File: firmware/App/Controllers/Heaters.c Revision Comment by Sean Nash on 09 November 2021, 13:20 https://devapps.diality.us/cru/DG-DEN-9480-1#c11161 Why is all of this code commented out? If not keeping this code, remove it. Reply by Dara Navaei on 09 November 2021, 16:01 > This is heaters monitor. This code is not going to be removed > but I have commented it to disable any monitoring while the > heaters are being monitored. I added a build switch for it. Reply by Sean Nash on 10 November 2021, 10:42 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 09 November 2021, 13:20 https://devapps.diality.us/cru/DG-DEN-9480-1#c11162 Why can't we monitor now? Reply by Dara Navaei on 09 November 2021, 16:03 > This part is under development and I have been working on the > happy path of the heaters. I added a build switch for now. Reply by Sean Nash on 10 November 2021, 10:41 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 09 November 2021, 13:24 https://devapps.diality.us/cru/DG-DEN-9480-1#c11165 No control - remove TODO. Reply by Dara Navaei on 09 November 2021, 16:09 > Done. Reply by Sean Nash on 10 November 2021, 10:39 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 09 November 2021, 13:23 https://devapps.diality.us/cru/DG-DEN-9480-1#c11163 Do we need an else? What should we do if we're in a non-heating mode? Reply by Dara Navaei on 09 November 2021, 16:05 > If we are in a non-heating mode, the heaters do not need to > be on and we should not be in this state. Reply by Sean Nash on 10 November 2021, 10:40 > RESOLVED in CODE WALKTHROUGH. Revision Comment by hnguyen on 09 November 2021, 14:30 https://devapps.diality.us/cru/DG-DEN-9480-1#c11198 In @details Inputs and Outputs, replace heatersVoltageMonitorTimeCounter with voltageMonitorTimeCounter. Reply by Dara Navaei on 09 November 2021, 20:17 > Done. Reply by hnguyen on 10 November 2021, 10:54 > RESOLVED in CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Services/SystemCommMessages.c Revision Comment by Sean Nash on 04 November 2021, 09:59 https://devapps.diality.us/cru/DG-DEN-9480-1#c11102 Can the two datas be memcpy'd as EVENT_DATA_Ts instead of U32 and EVENT_DATAS_T separately? Reply by Dara Navaei on 09 November 2021, 15:51 > Can this be optimized later? Reply by Sean Nash on 10 November 2021, 10:51 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/SystemCommMessages.h Revision Comment by Sean Nash on 04 November 2021, 09:58 https://devapps.diality.us/cru/DG-DEN-9480-1#c11101 Let's finish optimizing broadcasts. Reply by Dara Navaei on 06 November 2021, 16:37 > Done. Reply by Sean Nash on 10 November 2021, 10:51 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/Heaters.h Revision Comment by Sean Nash on 09 November 2021, 13:06 https://devapps.diality.us/cru/DG-DEN-9480-1#c11159 This pragma does not appear to be necessary. I have seen where structures with only F32s can have problems with this pragma. Reply by Dara Navaei on 09 November 2021, 15:59 > I removed it. Reply by Sean Nash on 10 November 2021, 10:42 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 09 November 2021, 13:05 https://devapps.diality.us/cru/DG-DEN-9480-1#c11158 Remove blank line. Reply by Dara Navaei on 09 November 2021, 15:58 > Done. Reply by Sean Nash on 10 November 2021, 10:42 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModeGenIdle.c Revision Comment by Sean Nash on 04 November 2021, 11:44 https://devapps.diality.us/cru/DG-DEN-9480-1#c11113 Should we keep the TODO for this? Reply by Dara Navaei on 06 November 2021, 16:04 > Added the TODO back. Reply by Sean Nash on 10 November 2021, 10:47 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 04 November 2021, 11:45 https://devapps.diality.us/cru/DG-DEN-9480-1#c11114 Should this be a DEBUG_DENALI comment so Bamboo will prevent a build? Reply by Dara Navaei on 06 November 2021, 16:03 > No it is uncommented. Reply by Sean Nash on 10 November 2021, 10:47 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModeInitPOST.c Revision Comment by Sean Nash on 04 November 2021, 11:26 https://devapps.diality.us/cru/DG-DEN-9480-1#c11111 Make a private definitions section above private data and move this there. Reply by Dara Navaei on 06 November 2021, 16:07 > Done. Reply by Sean Nash on 10 November 2021, 10:48 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/Fans.c Revision Comment by hnguyen on 09 November 2021, 13:58 https://devapps.diality.us/cru/DG-DEN-9480-1#c11192 In @details Outputs, remove "fansStatus" because I don't think we count the local variables as outputs Reply by Dara Navaei on 09 November 2021, 20:12 > Done. Reply by hnguyen on 10 November 2021, 10:55 > RESOLVED in CODE WALKTHROUGH Revision Comment by hnguyen on 09 November 2021, 14:17 https://devapps.diality.us/cru/DG-DEN-9480-1#c11196 In @details Inputs and Outputs, change dataPublishCounter to fansPublishCounter Reply by Dara Navaei on 09 November 2021, 20:13 > Done. Reply by hnguyen on 10 November 2021, 10:55 > RESOLVED in CODE WALKTHROUGH Revision Comment by hnguyen on 09 November 2021, 14:19 https://devapps.diality.us/cru/DG-DEN-9480-1#c11197 Add fansStatus to @details Inputs, since it is read at line 593 Reply by Dara Navaei on 09 November 2021, 20:16 > Done. Reply by hnguyen on 10 November 2021, 10:55 > RESOLVED in CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Modes/ModeDrain.c Revision Comment by Sean Nash on 05 November 2021, 11:39 https://devapps.diality.us/cru/DG-DEN-9480-1#c11134 We want 2400 mL/min. Will this RPM speed result in desired flow rate? Reply by Dara Navaei on 06 November 2021, 15:56 > With 2400 RPM the flow rate was ~2497.718 mL/min. I changed > the target RPM to 2300. Reply by Sean Nash on 10 November 2021, 10:45 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModeFill.c Revision Comment by Sean Nash on 05 November 2021, 11:08 https://devapps.diality.us/cru/DG-DEN-9480-1#c11129 What's happening here? Why subtract 0? Reply by Dara Navaei on 06 November 2021, 16:00 > Subtract 0.0 was unnecessary. I deleted it. Reply by Sean Nash on 10 November 2021, 10:46 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 05 November 2021, 11:13 https://devapps.diality.us/cru/DG-DEN-9480-1#c11130 We're saving fill data to RTC RAM every 50ms? Reply by Dara Navaei on 06 November 2021, 15:57 > This was not the right place for the function. I deleted the > function. Reply by Sean Nash on 10 November 2021, 10:46 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 05 November 2021, 11:15 https://devapps.diality.us/cru/DG-DEN-9480-1#c11131 Who would call this function? Reply by Dara Navaei on 06 November 2021, 15:57 > This function is no longer needed and I deleted it. Reply by Sean Nash on 10 November 2021, 10:45 > RESOLVED in CODE WALKTHROUGH. Revision Comment by hnguyen on 10 November 2021, 15:29 https://devapps.diality.us/cru/DG-DEN-9480-1#c11302 I am not sure why you removed the "const" from dialysateTemp and targetTemp? Reply by Dara Navaei on 10 November 2021, 21:06 > The const is an optional step in our code. Plus these are the > local variables and their values are not changed inside the > function. Reply by hnguyen on 11 November 2021, 10:36 > RESOLVED IN CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Modes/ModeHeatDisinfect.c Revision Comment by Sean Nash on 04 November 2021, 11:40 https://devapps.diality.us/cru/DG-DEN-9480-1#c11112 Does 2400 RPM get us 2400 mL/min drain rate? It's the rate that we want to be 2400. Reply by Dara Navaei on 06 November 2021, 16:06 > The drain flow rate does not matter in any of the disinfects. > The faster the drain, the better. Reply by Sean Nash on 10 November 2021, 10:47 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/FPGA.c Revision Comment by Sean Nash on 09 November 2021, 12:33 https://devapps.diality.us/cru/DG-DEN-9480-1#c11154 Remove blank line. Reply by Dara Navaei on 09 November 2021, 15:53 > This function is no longer needed. Reply by Sean Nash on 10 November 2021, 10:45 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/SystemComm.c Revision Comment by Sean Nash on 04 November 2021, 10:03 https://devapps.diality.us/cru/DG-DEN-9480-1#c11103 Make a build switch to disable ACK alarms. Reply by Dara Navaei on 06 November 2021, 16:36 > Done. Reply by Sean Nash on 10 November 2021, 10:50 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModeRecirculate.c Revision Comment by Sean Nash on 04 November 2021, 11:25 https://devapps.diality.us/cru/DG-DEN-9480-1#c11110 Should this module be removed (replaced by ModeGenIdle)? Reply by Dara Navaei on 06 November 2021, 16:22 > I do not see this mode my local code base. It might have been > git. Reply by Sean Nash on 10 November 2021, 10:48 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/Interrupts.c Revision Comment by Sean Nash on 04 November 2021, 10:33 https://devapps.diality.us/cru/DG-DEN-9480-1#c11107 This is removed in my branch. Reply by Dara Navaei on 10 November 2021, 11:06 > Removed it. Reply by Sean Nash on 10 November 2021, 11:16 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 04 November 2021, 10:08 https://devapps.diality.us/cru/DG-DEN-9480-1#c11105 Debug code is removed in my branch. Should s/w fault here I think. Reply by Dara Navaei on 10 November 2021, 11:09 > Done. Reply by Sean Nash on 10 November 2021, 11:17 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 04 November 2021, 10:34 https://devapps.diality.us/cru/DG-DEN-9480-1#c11108 Handling of passive notification is removed in my branch. Reply by Dara Navaei on 19 October 2023, 08:05 > RESOLVED in CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Controllers/ROPump.c Revision Comment by Sean Nash on 09 November 2021, 13:02 https://devapps.diality.us/cru/DG-DEN-9480-1#c11156 If pump is not on, do you need to call checkPersistentAlarm with FALSE to reset persistence? Reply by Dara Navaei on 09 November 2021, 15:56 > If the pump is not on, the monitor checks the voltage to make > sure it is reading 0. That code is a couple of lines above. Reply by Sean Nash on 09 November 2021, 16:44 > That if has an extra condition (beyond pump is not on). So > alarm persistence is only reset if other condition is met. > If other condition is not met, persistence is left in > limbo. Reply by Dara Navaei on 09 November 2021, 20:25 > I added an else to FALSE the persistent alarm. Reply by Sean Nash on 10 November 2021, 10:44 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 09 November 2021, 13:03 https://devapps.diality.us/cru/DG-DEN-9480-1#c11157 Why is this commented out? Reply by Dara Navaei on 09 November 2021, 15:57 > I uncommented it. Reply by Sean Nash on 10 November 2021, 10:43 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: NVDataMgmtDGRecords.h Revision Comment by Sean Nash on 04 November 2021, 09:48 https://devapps.diality.us/cru/DG-DEN-9480-1#c11100 There are now 2 DG flow sensors (FMP and FMD). Reply by Dara Navaei on 06 November 2021, 16:55 > The calibration of FMD has been added in DEN-9906 branch. Reply by Sean Nash on 10 November 2021, 10:52 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/DrainPump.c Revision Comment by hnguyen on 09 November 2021, 13:24 https://devapps.diality.us/cru/DG-DEN-9480-1#c11164 @details Inputs and Outputs need update Reply by Dara Navaei on 09 November 2021, 16:08 > Done. Reply by hnguyen on 10 November 2021, 10:59 > RESOLVED in CODE WALKTHROUGH Revision Comment by hnguyen on 09 November 2021, 13:26 https://devapps.diality.us/cru/DG-DEN-9480-1#c11166 targetDrainPumpSpeed has been changed in the code to targetDrainPumpRPM Reply by Dara Navaei on 10 November 2021, 11:01 > Done. Reply by hnguyen on 10 November 2021, 11:02 > RESOLVED in CODE WALKTHROUGH Revision Comment by hnguyen on 09 November 2021, 13:28 https://devapps.diality.us/cru/DG-DEN-9480-1#c11167 I do not see isRMPNonZeroInClosedLoop used as outputs in this function. Reply by Dara Navaei on 09 November 2021, 16:10 > I removed it. Reply by hnguyen on 10 November 2021, 10:57 > RESOLVED in CODE WALKTHROUGH Revision Comment by hnguyen on 09 November 2021, 13:31 https://devapps.diality.us/cru/DG-DEN-9480-1#c11168 @details Inputs and Outputs need update Reply by Dara Navaei on 09 November 2021, 16:22 > Done. Reply by hnguyen on 10 November 2021, 10:57 > RESOLVED in CODE WALKTHROUGH Revision Comment by hnguyen on 09 November 2021, 13:37 https://devapps.diality.us/cru/DG-DEN-9480-1#c11172 In @detail outputs, add signalNewRPMRequest Reply by Dara Navaei on 09 November 2021, 20:08 > Done. Reply by hnguyen on 10 November 2021, 10:57 > RESOLVED in CODE WALKTHROUGH Revision Comment by hnguyen on 09 November 2021, 13:43 https://devapps.diality.us/cru/DG-DEN-9480-1#c11177 drainPumpDAC on line 611 should be added to @detail inputs? Reply by Dara Navaei on 09 November 2021, 20:09 > Done. Reply by hnguyen on 10 November 2021, 10:57 > RESOLVED in CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Controllers/FluidLeak.c Revision Comment by Sean Nash on 09 November 2021, 13:11 https://devapps.diality.us/cru/DG-DEN-9480-1#c11160 Make state a U32 to ensure 4 byte size. Reply by Dara Navaei on 09 November 2021, 16:00 > Done. Reply by Sean Nash on 10 November 2021, 10:42 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/Pressures.c Revision Comment by hnguyen on 10 November 2021, 14:35 https://devapps.diality.us/cru/DG-DEN-9480-1#c11301 Added "pressuresDataPublicationTimerCounter" variable to @details Outputs Reply by Dara Navaei on 10 November 2021, 21:05 > Done. Reply by hnguyen on 11 November 2021, 10:36 > RESOLVED IN CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Controllers/Valves.c Revision Comment by hnguyen on 10 November 2021, 14:27 https://devapps.diality.us/cru/DG-DEN-9480-1#c11300 The state variable valvesStatesPublicationTimerCounter was read, increment, then write. Therefore, it needs to be added to the @details Outputs. Reply by Dara Navaei on 10 November 2021, 21:04 > Done. Reply by hnguyen on 11 November 2021, 10:37 > RESOLVED IN CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Controllers/Voltages.c Revision Comment by Sean Nash on 09 November 2021, 12:37 https://devapps.diality.us/cru/DG-DEN-9480-1#c11155 Should new RO pump speed signal be monitored? Reply by Dara Navaei on 09 November 2021, 15:54 > No, the voltage is monitored in the RO pump module. Reply by Sean Nash on 10 November 2021, 10:44 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/Reservoirs.c Revision Comment by hnguyen on 10 November 2021, 17:36 https://devapps.diality.us/cru/DG-DEN-9480-1#c11303 Variable reservoirDataPublicationTimerCounter is missing in the @detail Inputs and Output Reply by Dara Navaei on 10 November 2021, 21:08 > Done. Reply by hnguyen on 11 November 2021, 10:35 > RESOLVED IN CODE WALKTHROUGH ---------------------------------------- File: Accel.c Revision Comment by Sean Nash on 10 November 2021, 08:43 https://devapps.diality.us/cru/DG-DEN-9480-1#c11235 MSG id should be HD accel data Reply by Dara Navaei on 10 November 2021, 10:42 > Done. Reply by Sean Nash on 10 November 2021, 11:02 > RESOLVED in CODE WALKTHROUGH. --- ID: DG-DEN-9480-1 https://devapps.diality.us/cru/DG-DEN-9480-1 Title: DG-DEN-9480_DG_DEV Dialysate Temperature Control Statement of Objectives: State: Closed Summary: Author: Dara Navaei Moderator: Dara Navaei Reviewers: (1 active, 2 completed*) Sean Nash (*) hnguyen (*) Behrouz NematiPour