This is a list of all comments for HD-DEN-11114-1. Review Summary: No summary ---------------------------------------- File: firmware/App/Services/SystemCommMessages.c Revision Comment by Sean Nash on 29 November 2021, 16:45 https://devapps.diality.us/cru/HD-DEN-11114-1#c11474 This is a normal system message handler. It does not belong down in the Dialin test support area. Move up to normal handler functions area. Reply by Dara Navaei on 29 November 2021, 19:20 > Done. Reply by Sean Nash on 02 December 2021, 10:34 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/DGInterface.c Revision Comment by Sean Nash on 29 November 2021, 16:25 https://devapps.diality.us/cru/HD-DEN-11114-1#c11468 Why remove this later? Don't we need it? Reply by Dara Navaei on 02 December 2021, 10:36 > Done. Reply by Sean Nash on 02 December 2021, 10:36 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 29 November 2021, 16:25 https://devapps.diality.us/cru/HD-DEN-11114-1#c11469 Remove what? Reply by Dara Navaei on 29 November 2021, 19:21 > Done. Reply by Sean Nash on 02 December 2021, 10:36 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 29 November 2021, 16:26 https://devapps.diality.us/cru/HD-DEN-11114-1#c11470 Remove the enum? Did this get moved somewhere? Reply by Dara Navaei on 29 November 2021, 19:21 > Done. Reply by Sean Nash on 02 December 2021, 10:35 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/DGInterface.h Revision Comment by Sean Nash on 30 December 2021, 13:13 https://devapps.diality.us/cru/HD-DEN-11114-1#c11600 Update message list with latest broadcast msg data details. Reply by Dara Navaei on 03 January 2022, 10:14 > Done. Reply by Sean Nash on 04 January 2022, 10:27 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 29 November 2021, 16:24 https://devapps.diality.us/cru/HD-DEN-11114-1#c11467 Remove obsolete function prototypes. Reply by Dara Navaei on 29 November 2021, 19:22 > Done. Reply by Sean Nash on 02 December 2021, 10:36 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 30 December 2021, 13:14 https://devapps.diality.us/cru/HD-DEN-11114-1#c11601 Update message list with latest data details. Reply by Dara Navaei on 04 January 2022, 09:55 > Done. Reply by Sean Nash on 04 January 2022, 10:27 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/SystemCommMessages.h Revision Comment by Sean Nash on 29 November 2021, 16:41 https://devapps.diality.us/cru/HD-DEN-11114-1#c11472 ??? Reply by Dara Navaei on 29 November 2021, 19:17 > Cleaned up the message. Reply by Sean Nash on 02 December 2021, 10:34 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 29 November 2021, 16:43 https://devapps.diality.us/cru/HD-DEN-11114-1#c11473 This is a normal system message - should not be down here in the Dialin "test" message functions area. Reply by Dara Navaei on 29 November 2021, 19:20 > Done. Reply by Sean Nash on 02 December 2021, 10:34 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModeTreatment.c Revision Comment by Sean Nash on 29 November 2021, 16:37 https://devapps.diality.us/cru/HD-DEN-11114-1#c11471 I think it's time to remove this call. Reply by Dara Navaei on 29 November 2021, 19:21 > Done. Reply by Sean Nash on 02 December 2021, 10:35 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/BloodLeak.c Revision Comment by Sean Nash on 30 December 2021, 13:06 https://devapps.diality.us/cru/HD-DEN-11114-1#c11598 I would set this to at least 11 because you are using sprintf to create an unsigned integer string (max is 10 digits + 1 zero terminator) and we want to make sure we give sprintf a large enough buffer to populate - otherwise it will overwrite memory. Reply by Dara Navaei on 04 January 2022, 13:20 > Done. Reply by Sean Nash on 05 January 2022, 10:16 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 30 December 2021, 12:40 https://devapps.diality.us/cru/HD-DEN-11114-1#c11593 Recommend changing > to >= and then removing the - 1. Reply by Dara Navaei on 31 December 2021, 19:27 > Done. Reply by Sean Nash on 04 January 2022, 10:29 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 30 December 2021, 12:42 https://devapps.diality.us/cru/HD-DEN-11114-1#c11594 .crc? Reply by Dara Navaei on 31 December 2021, 19:25 > Fixed it. Reply by Sean Nash on 04 January 2022, 10:29 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 30 December 2021, 12:43 https://devapps.diality.us/cru/HD-DEN-11114-1#c11595 Create and trigger new alarm. Reply by Dara Navaei on 31 December 2021, 19:25 > Done. Reply by Sean Nash on 04 January 2022, 10:29 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 30 December 2021, 12:59 https://devapps.diality.us/cru/HD-DEN-11114-1#c11596 1 and 0 is not clear. Better to say FIFO send and reset. Reply by Dara Navaei on 31 December 2021, 19:25 > Done. Reply by Sean Nash on 04 January 2022, 10:29 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 30 December 2021, 13:08 https://devapps.diality.us/cru/HD-DEN-11114-1#c11599 We don't need to calculate digit count - just use strlen(tempCharBuffer) after sprintf creates your string. Reply by Dara Navaei on 04 January 2022, 10:01 > Done. Reply by Sean Nash on 04 January 2022, 10:28 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 30 December 2021, 13:03 https://devapps.diality.us/cru/HD-DEN-11114-1#c11597 .crc? Reply by Dara Navaei on 31 December 2021, 19:22 > Done. Reply by Sean Nash on 04 January 2022, 10:29 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: Accel.c Revision Comment by Sean Nash on 03 January 2022, 08:44 https://devapps.diality.us/cru/HD-DEN-11114-1#c11607 Align comment. Reply by Dara Navaei on 03 January 2022, 09:28 > Done. Reply by Sean Nash on 04 January 2022, 10:27 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/BloodLeak.h Revision Comment by Sean Nash on 30 December 2021, 12:28 https://devapps.diality.us/cru/HD-DEN-11114-1#c11592 Please update message list details for this broadcast msg. Reply by Dara Navaei on 03 January 2022, 09:45 > Done. Reply by Sean Nash on 04 January 2022, 10:30 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: NVDataMgmt.c Revision Comment by Sean Nash on 30 December 2021, 12:20 https://devapps.diality.us/cru/HD-DEN-11114-1#c11589 Is CRC check on individual cal section done elsewhere or should it be considered here as well? Reply by Dara Navaei on 03 January 2022, 09:35 > No I do not think so. The CRCs are all checked in the self > test. Reply by Sean Nash on 04 January 2022, 10:31 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 30 December 2021, 12:22 https://devapps.diality.us/cru/HD-DEN-11114-1#c11590 We should verify bufferLength is >= to cal section byte size before doing this memcpy - otherwise, we will be overwriting memory. Reply by Dara Navaei on 03 January 2022, 09:31 > Done. Reply by Sean Nash on 04 January 2022, 10:30 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 30 December 2021, 12:24 https://devapps.diality.us/cru/HD-DEN-11114-1#c11591 If h/w component is not required to have a calibration, we shouldn't alarm here - right? Reply by Dara Navaei on 03 January 2022, 09:31 > Yes, if you add "alarm no alarm" it will not be checked. Reply by Sean Nash on 04 January 2022, 10:30 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 03 January 2022, 08:52 https://devapps.diality.us/cru/HD-DEN-11114-1#c11608 Magic numbers on these new for loops. Reply by Dara Navaei on 04 January 2022, 09:45 > Done. Reply by Sean Nash on 04 January 2022, 10:27 > RESOLVED in CODE WALKTHROUGH. --- ID: HD-DEN-11114-1 https://devapps.diality.us/cru/HD-DEN-11114-1 Title: HD-DEN-11114_Dialysate Temperature Control (2 of 2) Statement of Objectives: State: Closed Summary: Author: Dara Navaei Moderator: Dara Navaei Reviewers: (1 active, 2 completed*) Sean Nash (*) hnguyen (*) Behrouz NematiPour