This is a list of all comments for LEAHI-DD-FIRMWARE-LDT-2004-2. Review Summary: No summary ---------------------------------------- File: firmware/App/Drivers/ConductivityTeensy.c Revision Comment by Vendor - TEL - Sameer Poyil on 18 November 2025, 16:27 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2#c25174 Follow alphabetical order of including header file Reply by Sean Nash on 20 November 2025, 08:53 > I think this is ok as is. We do want includes in > alphabetical order, but in groups: 1) library header files, > 2) HAL header files, and 3) our header files. Revision Comment by Vendor - TEL - Sameer Poyil on 18 November 2025, 16:29 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2#c25175 Need doxygen comments for all of the parameter Revision Comment by Vendor - TEL - Sameer Poyil on 18 November 2025, 16:32 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2#c25176 align the comments Revision Comment by Vendor - TEL - Sameer Poyil on 18 November 2025, 10:10 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2#c25166 give comments for all parameters Revision Comment by Sean Nash on 20 November 2025, 08:54 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2#c25195 Need doxygen comment. Revision Comment by Vendor - TEL - Sameer Poyil on 11 November 2025, 07:37 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2#c25058 use uniform naming convention. All other places used conductivity Revision Comment by Sean Nash on 20 November 2025, 08:55 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2#c25196 All functions need function headers. Revision Comment by Vendor - TEL - Sameer Poyil on 18 November 2025, 10:02 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2#c25164 Do not use the magic number. Revision Comment by Vendor - TEL - Sameer Poyil on 18 November 2025, 16:39 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2#c25177 as per coding guide line , it should be tested against the correct value , TRUE or FALSE for example if ((bufsize ) == 0) Reply by Sean Nash on 20 November 2025, 08:56 > Prefer something like if ( FALSE == isQueueFull() ). Revision Comment by Vendor - TEL - Sameer Poyil on 18 November 2025, 16:42 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2#c25178 it should test all the cases like for example if (written >= 0 && written < sizeof(buffer)) { // Success the string was fully written and null-terminated } else if (written >= sizeof(buffer)) { // The output was truncated. handle as needed } else { // An encoding error occurred } Revision Comment by Vendor - TEL - Sameer Poyil on 18 November 2025, 10:03 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2#c25165 memcpy is unsafe function. check if there is safer version of memcpy like memcpy_s Reply by Sean Nash on 20 November 2025, 09:02 > I don't see support for that with our compiler. Author of > code must ensure destination buffer is large enough to hold > the specified number of bytes being copied. Code reviewer > should verify this. Revision Comment by Sean Nash on 20 November 2025, 09:05 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2#c25199 Why are we doing pre-increment instead of post-increment? I don't think it matters in for loop, but this looks non-standard to me. Revision Comment by Vendor - TEL - Sameer Poyil on 18 November 2025, 19:41 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2#c25180 space required starting and end of switch input parameter Revision Comment by Vendor - TEL - Sameer Poyil on 18 November 2025, 12:53 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2#c25170 comparing signed and unsigned value. make FLOAT_COUNT as unsigned. Not checking the return value of snprintf if there is any error happened or not. Revision Comment by Vendor - TEL - Sameer Poyil on 18 November 2025, 19:44 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2#c25181 space required at starting and end of the function parameters Revision Comment by Vendor - TEL - Sameer Poyil on 18 November 2025, 19:47 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2#c25183 space required. check all other if condition and while statement Revision Comment by Vendor - TEL - Sameer Poyil on 18 November 2025, 19:37 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2#c25179 Always check the return value of snprintf to detect potential errors Reply by Sean Nash on 20 November 2025, 09:16 > Agree, but this is a temporary driver so I don't want to > spend too much time making this unit perfect. Revision Comment by Vendor - TEL - Sameer Poyil on 18 November 2025, 19:46 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2#c25182 space required ---------------------------------------- File: firmware/App/Drivers/ConductivityTeensy.h Revision Comment by Vendor - TEL - Sameer Poyil on 18 November 2025, 13:33 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2#c25171 There shouldn't be a name if using typedef Revision Comment by Vendor - TEL - Sameer Poyil on 11 November 2025, 07:34 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2#c25057 move all structure alignment in to a single block #pragma pack(push,1) and #pragma(pop) ---------------------------------------- File: firmware/App/Drivers/ConductivitySensors.c Revision Comment by Sean Nash on 12 November 2025, 14:58 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2#c25088 Why? Revision Comment by Sean Nash on 12 November 2025, 14:58 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2#c25090 Move to ConductivityTeensy.c? Revision Comment by Sean Nash on 12 November 2025, 14:59 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2#c25091 Why? Revision Comment by Vendor - TEL - Sameer Poyil on 18 November 2025, 16:21 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2#c25172 Switch case is better than if else Revision Comment by Vendor - TEL - Sameer Poyil on 18 November 2025, 16:22 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2#c25173 switch case is better than if else. check for all similar implementation. Revision Comment by Sean Nash on 12 November 2025, 14:59 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2#c25092 Why here? ---------------------------------------- File: firmware/App/Services/Interrupts.c Revision Comment by Sean Nash on 12 November 2025, 15:00 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2#c25093 #ifdef USE_TEENSY_CONDUCTIVITY_DRIVER Revision Comment by Sean Nash on 12 November 2025, 15:00 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2#c25094 #ifdef ---------------------------------------- File: firmware/App/Monitors/Conductivity.c Revision Comment by Sean Nash on 12 November 2025, 15:01 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2#c25095 #ifdef Revision Comment by Sean Nash on 20 November 2025, 09:19 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2#c25202 #ifdef Revision Comment by Sean Nash on 12 November 2025, 15:01 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2#c25096 #ifdef ---------------------------------------- File: firmware/App/Controllers/Heaters.c Revision Comment by Sean Nash on 20 November 2025, 08:48 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2#c25193 Should we be calling a get function in Conductivity.c (monitor) instead of driver? Monitor would give a filtered value and driver would give a raw value. I would think only the monitor would want the raw value. ---------------------------------------- File: firmware/App/DDCommon.h Revision Comment by Sean Nash on 20 November 2025, 09:23 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2#c25204 Comment seems backward. Uncommenting would enable the Teensy driver, right? ---------------------------------------- File: firmware/App/Modes/ModeGenDialysate.c Revision Comment by Sean Nash on 20 November 2025, 09:18 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2#c25201 Should be calling get function in monitor (not driver). ---------------------------------------- File: firmware/App/Services/Messaging.c Revision Comment by Sean Nash on 20 November 2025, 09:21 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2#c25203 Are we not supporting overrides with TEENSY driver? --- ID: LEAHI-DD-FIRMWARE-LDT-2004-2 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2004-2 Title: LEAHI-DD-FIRMWARE-LDT-2004_Dialysate Composition - DD Statement of Objectives: State: Review Author: Michael Garthwaite Moderator: Michael Garthwaite Reviewers: (10 active, 0 completed*) Vendor - TEL - Jashwant Gantyada Vendor - TEL - Arpita Srivastava Sean Nash Vendor - TEL - Varshini Nagabooshanam Vinayakam Mani Raghu Kallala Dara Navaei Vendor - TEL - Sivvanarayana Kurapati Daniel Ho Vendor - TEL - Sameer Poyil