This is a list of all comments for HD-DEN-4640-1. Review Summary: No summary ---------------------------------------- File: firmware/App/Services/SystemCommMessages.h Revision Comment by pmontazemi on 20 October 2020, 13:19 https://devapps.diality.us/cru/HD-DEN-4640-1#c5686 1. Why dialInData is not a pointer but line below *dialOutFlowData is pointed? 2. Why one's name does not include the keyword "Flow" and the other does? Reply by Sean Nash on 26 October 2020, 09:25 > Not consistent. Made all pointers. Reply by pmontazemi on 26 October 2020, 10:31 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/BloodFlow.c Revision Comment by pmontazemi on 19 October 2020, 10:14 https://devapps.diality.us/cru/HD-DEN-4640-1#c5576 Remove extra line. Reply by Sean Nash on 19 October 2020, 10:24 > Done. Reply by pmontazemi on 19 October 2020, 10:35 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/DialInFlow.c Revision Comment by pmontazemi on 20 October 2020, 11:27 https://devapps.diality.us/cru/HD-DEN-4640-1#c5676 Why one flow is in percent and the other in per unit? (which requires conversion) Reply by Sean Nash on 20 October 2020, 11:51 > In f/w, I like to keep percentages as per unit so that the > math works - e.g. if I need to apply percentage to something > I can just multiply it directly. > On Dialin side, I kept as percentage. Reply by pmontazemi on 26 October 2020, 10:36 > RESOLVED in CODE WALKTHROUGH. Revision Comment by pmontazemi on 20 October 2020, 11:27 https://devapps.diality.us/cru/HD-DEN-4640-1#c5677 Why one value is in percent and the other in per unit? (which requires conversion) Reply by Sean Nash on 20 October 2020, 11:54 > In f/w, I like to keep percentages as per unit so that the > math works - e.g. if I need to apply percentage to something > I can just multiply it directly. > On Dialin side, I kept as percentage. Reply by pmontazemi on 26 October 2020, 10:36 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/DialOutFlow.h Revision Comment by qnguyen on 12 October 2020, 09:25 https://devapps.diality.us/cru/HD-DEN-4640-1#c5431 Suggest expanding these macros. Reply by Sean Nash on 12 October 2020, 10:19 > Done. Reply by qnguyen on 14 October 2020, 10:33 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/HDCommon.h Revision Comment by pmontazemi on 20 October 2020, 13:20 https://devapps.diality.us/cru/HD-DEN-4640-1#c5687 Align parameters and associated comments. Reply by Sean Nash on 26 October 2020, 09:22 > These will always be a mix of some un-commented and some > commented. Comments would be aligned if all were > un-commented. Reply by pmontazemi on 26 October 2020, 10:31 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/FPGA.c Revision Comment by pmontazemi on 05 October 2020, 15:33 https://devapps.diality.us/cru/HD-DEN-4640-1#c5206 Add extra line before if. Reply by Sean Nash on 05 October 2020, 15:41 > Done. Reply by pmontazemi on 05 October 2020, 16:15 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: Utilities.c Revision Comment by qnguyen on 05 October 2020, 09:23 https://devapps.diality.us/cru/HD-DEN-4640-1#c5141 Function name is not matched. Reply by Sean Nash on 05 October 2020, 10:26 > Done. Reply by qnguyen on 05 October 2020, 16:14 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: AlarmDefs.h Revision Comment by pmontazemi on 05 October 2020, 15:25 https://devapps.diality.us/cru/HD-DEN-4640-1#c5204 Why here is 86 and in Dialin is 88? Reply by Sean Nash on 05 October 2020, 15:42 > F/W would be 87 here if we numbered the last > "NUM_OF_ALARM_IDS" like Dialin did. And then Dialin has a > new valve alarm ID that Dara added so that branch is a little > bit ahead of the f/w branch. The alarm IDs are kept in a > common repo for all 3 stacks and is shared by all so it can > get updated by somebody else. Reply by pmontazemi on 05 October 2020, 16:15 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModeTreatment.c Revision Comment by pmontazemi on 20 October 2020, 13:12 https://devapps.diality.us/cru/HD-DEN-4640-1#c5685 Align all parameters and all corresponding Doxygen comments. Reply by Sean Nash on 26 October 2020, 09:28 > Done. Reply by pmontazemi on 26 October 2020, 10:31 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/PresOccl.c Revision Comment by pmontazemi on 20 October 2020, 13:01 https://devapps.diality.us/cru/HD-DEN-4640-1#c5683 Add units in comment between parentheses. Reply by Sean Nash on 26 October 2020, 10:40 > Done. Reply by pmontazemi on 26 October 2020, 10:41 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/Valves.h Revision Comment by pmontazemi on 20 October 2020, 13:04 https://devapps.diality.us/cru/HD-DEN-4640-1#c5684 Purpose of asking pre-processor to pack? Reply by Sean Nash on 26 October 2020, 09:29 > Structure fields are a mix of different sizes. Compiler will > put each field in 32-bit space and pad by default. Since we > will be serializing this data for transmission, we do not > want structure to include any padding. The pack pragma > instructs compiler not to pad. Reply by pmontazemi on 26 October 2020, 10:32 > RESOLVED in CODE WALKTHROUGH. --- ID: HD-DEN-4640-1 https://devapps.diality.us/cru/HD-DEN-4640-1 Title: HD-DEN-4640_Saline Bolus Statement of Objectives: State: Closed Summary: Author: Sean Nash Reviewers: (0 active, 3 completed*) qnguyen (*) Dara Navaei (*) pmontazemi (*)