This is a list of all comments for HD-DEN-4211-1. Review Summary: No summary ---------------------------------------- File: firmware/App/Modes/ModeTreatmentParams.c Revision Comment by pmontazemi on 28 August 2020, 11:22 https://devapps.diality.us/cru/HD-DEN-4211-1#c4149 Anything messaging we want to do when SW fault? Reply by Sean Nash on 28 August 2020, 16:04 > So this would indicate we're in an invalid state for > treatment parameters mode which shouldn't be possible unless > memory was corrupted (stack overflow or buffer overflow, > ...). So we will trigger a s/w fault (generic something bad > happened in s/w fault) with supporting data of 1) a unique # > (enum) to indicate this s/w fault is an invalid treatment > parameters mode state and 2) what was the invalid state ID > that s/w saw. All of that will be logged by UI. This is all > part of the TODO which I have addressed in next commit. Reply by pmontazemi on 28 August 2020, 16:38 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 26 August 2020, 11:04 https://devapps.diality.us/cru/HD-DEN-4211-1#c3973 Change to doxygen style. Reply by Sean Nash on 26 August 2020, 12:16 > Done Reply by qnguyen on 28 August 2020, 16:49 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 02 September 2020, 15:54 https://devapps.diality.us/cru/HD-DEN-4211-1#c4383 Suggesting move result assignment out since the result is same for all cases. Reply by Sean Nash on 02 September 2020, 17:32 > Result no longer same. Reply by qnguyen on 08 September 2020, 10:34 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModeTreatmentParams.h Revision Comment by qnguyen on 26 August 2020, 11:25 https://devapps.diality.us/cru/HD-DEN-4211-1#c3990 Change private to public. Reply by Sean Nash on 26 August 2020, 12:08 > Done Reply by qnguyen on 28 August 2020, 16:43 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/DialOutFlow.c Revision Comment by qnguyen on 26 August 2020, 11:15 https://devapps.diality.us/cru/HD-DEN-4211-1#c3978 Remove function name. Reply by Sean Nash on 26 August 2020, 12:13 > Done Reply by qnguyen on 28 August 2020, 16:48 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 26 August 2020, 11:15 https://devapps.diality.us/cru/HD-DEN-4211-1#c3979 Remove function name. Reply by Sean Nash on 26 August 2020, 12:13 > Done Reply by qnguyen on 28 August 2020, 16:47 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 26 August 2020, 11:15 https://devapps.diality.us/cru/HD-DEN-4211-1#c3980 Remove function name. Reply by Sean Nash on 26 August 2020, 12:12 > Done Reply by qnguyen on 28 August 2020, 16:47 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 26 August 2020, 11:15 https://devapps.diality.us/cru/HD-DEN-4211-1#c3981 Remove function name. Reply by Sean Nash on 26 August 2020, 12:12 > Done Reply by qnguyen on 28 August 2020, 16:47 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 26 August 2020, 11:15 https://devapps.diality.us/cru/HD-DEN-4211-1#c3982 Remove function name. Reply by Sean Nash on 26 August 2020, 12:12 > Done Reply by qnguyen on 28 August 2020, 16:47 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 26 August 2020, 11:15 https://devapps.diality.us/cru/HD-DEN-4211-1#c3983 Remove function name. Reply by Sean Nash on 26 August 2020, 12:12 > Done Reply by qnguyen on 28 August 2020, 16:47 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 26 August 2020, 11:16 https://devapps.diality.us/cru/HD-DEN-4211-1#c3984 Remove function name. Reply by Sean Nash on 26 August 2020, 12:11 > Done Reply by qnguyen on 28 August 2020, 16:46 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 26 August 2020, 10:44 https://devapps.diality.us/cru/HD-DEN-4211-1#c3961 Suggest removing -1 and change to less than "<". Reply by Sean Nash on 26 August 2020, 12:30 > Done Reply by qnguyen on 28 August 2020, 16:47 > RESOLVED in CODE WALKTHROUGH. Revision Comment by pmontazemi on 28 August 2020, 10:42 https://devapps.diality.us/cru/HD-DEN-4211-1#c4111 Why is rotation direction is set in this function? Is there a need to define positive and negative flows as part of flow rate override? Reply by Sean Nash on 28 August 2020, 16:17 > So this is a Dialin handler to command the pump to a new set > point (flow rate). I suppose it could have taken an unsigned > rate and a separate direction parameter. Combining direction > with target rate as a signed value reduced message size from > Dialin. The Dialin command is commented to note that a > negative rate indicates reverse direction. Reply by pmontazemi on 28 August 2020, 16:41 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/HDCommon.h Revision Comment by pmontazemi on 28 August 2020, 13:04 https://devapps.diality.us/cru/HD-DEN-4211-1#c4173 Remove all commented lines, or add TODOs. Reply by Sean Nash on 28 August 2020, 15:28 > I amended comment to clarify these are all for development > builds only - Bamboo will never see any of these. > As for which dev build switches are commented out or not, > this is just how they are used. You un-comment a switch to > get that special test feature/behavior as needed. You > comment it out to turn it off. > So this isn't TODO situation. It's just a means to > facilitate all of the various test build requests I get. Reply by pmontazemi on 28 August 2020, 16:34 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/SystemCommMessages.c Revision Comment by pmontazemi on 28 August 2020, 12:49 https://devapps.diality.us/cru/HD-DEN-4211-1#c4171 Why removed? Reply by Sean Nash on 28 August 2020, 15:38 > Moved to MessagePayloads.h for now to reduce code in an > already large module. These payload definitions will > eventually be distributed to the various modules that will be > sending/handling the associated messages. Reply by pmontazemi on 28 August 2020, 16:36 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 26 August 2020, 11:06 https://devapps.diality.us/cru/HD-DEN-4211-1#c3974 Change to doxygen style. Reply by Sean Nash on 26 August 2020, 12:15 > Done Reply by qnguyen on 28 August 2020, 16:49 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 26 August 2020, 11:07 https://devapps.diality.us/cru/HD-DEN-4211-1#c3975 Change to doxygen style. Reply by Sean Nash on 26 August 2020, 12:15 > Done Reply by qnguyen on 28 August 2020, 16:49 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 26 August 2020, 10:13 https://devapps.diality.us/cru/HD-DEN-4211-1#c3940 Consider changing 0 to FALSE. Reply by Sean Nash on 26 August 2020, 12:34 > Done Reply by qnguyen on 28 August 2020, 16:52 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/SystemComm.c Revision Comment by qnguyen on 26 August 2020, 10:54 https://devapps.diality.us/cru/HD-DEN-4211-1#c3963 Replace # with number. Reply by Sean Nash on 28 August 2020, 16:55 > Done Reply by qnguyen on 31 August 2020, 16:18 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 26 August 2020, 10:55 https://devapps.diality.us/cru/HD-DEN-4211-1#c3964 Replace # with number. Reply by Sean Nash on 26 August 2020, 12:29 > Done Reply by qnguyen on 28 August 2020, 16:50 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 26 August 2020, 10:55 https://devapps.diality.us/cru/HD-DEN-4211-1#c3965 Replace # with number. Reply by Sean Nash on 26 August 2020, 12:28 > Done Reply by qnguyen on 28 August 2020, 16:50 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 26 August 2020, 11:10 https://devapps.diality.us/cru/HD-DEN-4211-1#c3976 Change msg buffer to buffer and remove semi-colon. Reply by Sean Nash on 26 August 2020, 12:15 > Done. Reply by qnguyen on 28 August 2020, 16:48 > RESOLVED in CODE WALKTHROUGH. Revision Comment by pmontazemi on 28 August 2020, 12:48 https://devapps.diality.us/cru/HD-DEN-4211-1#c4169 Why removed? Reply by Sean Nash on 28 August 2020, 15:47 > This was an old idea I'd had about something we might want > Dialin to be able to do, but seeing how Dialin has evolved > it's clear we will not be doing this. Reply by pmontazemi on 28 August 2020, 16:38 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/AlarmMgmt.c Revision Comment by qnguyen on 26 August 2020, 10:42 https://devapps.diality.us/cru/HD-DEN-4211-1#c3959 Suggest removing -1 and change to less than "<". Reply by Sean Nash on 26 August 2020, 12:31 > Done Reply by qnguyen on 28 August 2020, 16:51 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/CommBuffers.c Revision Comment by pmontazemi on 28 August 2020, 12:39 https://devapps.diality.us/cru/HD-DEN-4211-1#c4168 One liner Reply by Sean Nash on 28 August 2020, 16:03 > Done Reply by pmontazemi on 28 August 2020, 16:38 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 26 August 2020, 10:56 https://devapps.diality.us/cru/HD-DEN-4211-1#c3970 Replace # with number. Reply by Sean Nash on 26 August 2020, 12:17 > Done Reply by qnguyen on 28 August 2020, 16:49 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/CommBuffers.h Revision Comment by qnguyen on 26 August 2020, 10:25 https://devapps.diality.us/cru/HD-DEN-4211-1#c3944 Is this an intent extra space between "data." and "A"? Reply by Sean Nash on 26 August 2020, 12:33 > No. Removed. Reply by qnguyen on 28 August 2020, 16:51 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/WatchdogMgmt.c Revision Comment by qnguyen on 26 August 2020, 10:14 https://devapps.diality.us/cru/HD-DEN-4211-1#c3941 Align doxygen comments. Reply by Sean Nash on 28 August 2020, 16:57 > Done Reply by qnguyen on 31 August 2020, 16:19 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 26 August 2020, 10:38 https://devapps.diality.us/cru/HD-DEN-4211-1#c3956 Suggest removing -1 and change to less than "<". Reply by Sean Nash on 26 August 2020, 12:32 > Done Reply by qnguyen on 28 August 2020, 16:51 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModeStandby.h Revision Comment by qnguyen on 26 August 2020, 11:24 https://devapps.diality.us/cru/HD-DEN-4211-1#c3989 Change private to public. Reply by Sean Nash on 26 August 2020, 12:09 > Done Reply by qnguyen on 28 August 2020, 16:44 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/BloodFlow.c Revision Comment by qnguyen on 26 August 2020, 11:12 https://devapps.diality.us/cru/HD-DEN-4211-1#c3977 Remove function name. Reply by Sean Nash on 26 August 2020, 12:13 > Done Reply by qnguyen on 28 August 2020, 16:48 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModeFault.c Revision Comment by pmontazemi on 28 August 2020, 11:06 https://devapps.diality.us/cru/HD-DEN-4211-1#c4148 Is this position value expected to change? Reply by Sean Nash on 28 August 2020, 16:13 > All of this is EMC test code per #ifdef above (~line 75) so > this code is going away soon. Reply by pmontazemi on 28 August 2020, 16:39 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModeInitPOST.c Revision Comment by qnguyen on 26 August 2020, 11:17 https://devapps.diality.us/cru/HD-DEN-4211-1#c3985 Change to doxygen style and remove function name. Reply by Sean Nash on 26 August 2020, 12:11 > Done Reply by qnguyen on 28 August 2020, 16:46 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 26 August 2020, 11:17 https://devapps.diality.us/cru/HD-DEN-4211-1#c3986 Change to doxygen style and remove function name. Reply by Sean Nash on 26 August 2020, 12:10 > Done Reply by qnguyen on 28 August 2020, 16:46 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/OperationModes.c Revision Comment by qnguyen on 26 August 2020, 11:18 https://devapps.diality.us/cru/HD-DEN-4211-1#c3988 Remove function name from @brief and rename function name to match. Reply by Sean Nash on 26 August 2020, 12:09 > Done Reply by qnguyen on 28 August 2020, 16:45 > Rename function name to match. Reply by Sean Nash on 28 August 2020, 16:55 > Done Reply by qnguyen on 31 August 2020, 16:18 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 26 August 2020, 11:17 https://devapps.diality.us/cru/HD-DEN-4211-1#c3987 Remove function name. Reply by Sean Nash on 26 August 2020, 12:10 > Done Reply by qnguyen on 28 August 2020, 16:46 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/FPGA.c Revision Comment by qnguyen on 26 August 2020, 10:55 https://devapps.diality.us/cru/HD-DEN-4211-1#c3966 Replace # with number. Reply by Sean Nash on 28 August 2020, 16:55 > Done Reply by qnguyen on 31 August 2020, 16:18 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 26 August 2020, 10:55 https://devapps.diality.us/cru/HD-DEN-4211-1#c3967 Replace # with number. Reply by Sean Nash on 26 August 2020, 12:28 > Done Reply by qnguyen on 28 August 2020, 16:50 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 26 August 2020, 10:56 https://devapps.diality.us/cru/HD-DEN-4211-1#c3969 Replace # with number. Reply by Sean Nash on 26 August 2020, 12:18 > Done Reply by qnguyen on 28 August 2020, 16:50 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 26 August 2020, 10:55 https://devapps.diality.us/cru/HD-DEN-4211-1#c3968 Replace # with number. Reply by Sean Nash on 26 August 2020, 12:19 > Done Reply by qnguyen on 28 August 2020, 16:50 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 26 August 2020, 10:24 https://devapps.diality.us/cru/HD-DEN-4211-1#c3942 Add doxygen header for below functions. Reply by Sean Nash on 26 August 2020, 12:27 > Done Reply by qnguyen on 28 August 2020, 16:51 > RESOLVED in CODE WALKTHROUGH. Revision Comment by pmontazemi on 09 September 2020, 15:32 https://devapps.diality.us/cru/HD-DEN-4211-1#c4672 Why did get this section deleted? Reply by Sean Nash on 09 September 2020, 16:05 > This is from Dara's HD valves branch. I pulled his FPGA > module because he did the last update for register alignment > with latest FPGA version, but now that I'm trying to wrap up > dev testing, I don't want to duplicate effort to test these > functions. All I really needed from the pull was the > register map - should have deleted these right after the > pull. Reply by pmontazemi on 14 September 2020, 14:37 > RESOLVED in CODE WALKTHROUGH. Revision Comment by qnguyen on 26 August 2020, 10:24 https://devapps.diality.us/cru/HD-DEN-4211-1#c3943 Add blank line between two functions. Reply by Sean Nash on 26 August 2020, 12:27 > Done Reply by qnguyen on 28 August 2020, 16:52 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/Buttons.h Revision Comment by qnguyen on 26 August 2020, 11:30 https://devapps.diality.us/cru/HD-DEN-4211-1#c3995 Missing one slash /. Reply by Sean Nash on 26 August 2020, 12:05 > Done Reply by qnguyen on 28 August 2020, 16:43 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Tasks/TaskPriority.c Revision Comment by pmontazemi on 28 August 2020, 13:03 https://devapps.diality.us/cru/HD-DEN-4211-1#c4172 disable accels? Or, enabled accels? Reply by Sean Nash on 28 August 2020, 15:35 > I guess this is a double negative so it's confusing, but I > try to keep the normal state first in these #if build > switches. > So this one is a #ifndef which is the normal path where > accelerometer feature is enabled. And so the #else is the > abnormal state where accelerometer is disabled. Reply by pmontazemi on 28 August 2020, 16:36 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/BloodFlow.h Revision Comment by qnguyen on 26 August 2020, 11:26 https://devapps.diality.us/cru/HD-DEN-4211-1#c3991 Do we want to expand all these macros? Reply by Sean Nash on 26 August 2020, 12:08 > Yes. Done. Reply by qnguyen on 28 August 2020, 16:43 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/SystemComm.h Revision Comment by pmontazemi on 28 August 2020, 12:49 https://devapps.diality.us/cru/HD-DEN-4211-1#c4170 Align all comments under each other vertically. Reply by Sean Nash on 28 August 2020, 15:46 > Done Reply by pmontazemi on 28 August 2020, 16:37 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: Accel.c Revision Comment by pmontazemi on 09 September 2020, 14:51 https://devapps.diality.us/cru/HD-DEN-4211-1#c4664 Indent everything to align. Reply by Sean Nash on 09 September 2020, 15:25 > Done. Reply by pmontazemi on 14 September 2020, 14:37 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: Utilities.h Revision Comment by qnguyen on 26 August 2020, 11:36 https://devapps.diality.us/cru/HD-DEN-4211-1#c3996 Change # to number. Reply by Sean Nash on 26 August 2020, 12:05 > Done Reply by qnguyen on 28 August 2020, 16:42 > RESOLVED in CODE WALKTHROUGH. --- ID: HD-DEN-4211-1 https://devapps.diality.us/cru/HD-DEN-4211-1 Title: HD-DEN-4211_HD Treatment Parameters Mode Statement of Objectives: State: Closed Summary: Author: Sean Nash Moderator: Sean Nash Reviewers: (0 active, 3 completed*) qnguyen (*) Dara Navaei (*) pmontazemi (*)