•  

Comment Results

Review Name Created Custom Fields Content
DIALIN-DEN-11980-1 22 Feb 2022

RESOLVED.

UI-DEN-10205-1 22 Dec 2021

RESOLVED

HD-DEN-13834-1 17 Oct 2022

RESOLVED IN CODE WALKTHROUGH

UI-DEN-10602-1 17 Dec 2021

Fixed.
Thanks for your attention to the details.

DG-DEN-11750-1 22 Feb 2022

Notices the 170 files that need to be reviewed

https://c.tenor.com/RXMxqsRKEn0AAAAC/this-is-fine.gif

DG-DEN-5963-1 13 Apr 2021

This struct is the structure that is used to set the dialysate target temperature using the CAN bus. I do not think we can use enums for this purpose.

DIALIN-DEN-11750-1 23 Feb 2022

This script is in the tests folder. The tests folder is not controlled in the way the Dialin files are controlled so I might go back and forth in between different tests. This is not affecting any of the Dialin features.

HD-DEN-13834-1 17 Oct 2022

This is an old code and it has been removed.

UI-DEN-10205-1 22 Dec 2021

RESOLVED

DG-DEN-5963-1 13 Apr 2021

The small and main primary duty cycles are on different PWM channels. They cannot be controlled with one function.

DIALIN-DEN-7135-1 13 Apr 2021

RESOLVED.

DIALIN-DEN-11750-1 23 Feb 2022

This script is in the tests folder. The tests folder is not controlled in the way the Dialin files are controlled so I might go back and forth in between different tests. This is not affecting any of the Dialin features.

UI-DEN-7044-1 13 Apr 2021

RESOLVED

DG-DEN-5963-1 14 Apr 2021

Done.

HD-DEN-7395-1 13 Apr 2021

I think better to use self test status enum (mentioned in .h comment). This enum will have pass/fail/in progress. I think if we're still in INIT, zero, or self-test state, in-progress is appropriate return value. I think you should have a BOOL variable defined to remember whether self-test passed or failed (instead of inferring from current state). Then if in normal state, return pass if self-test passed, failed if not.

DIALIN-DEN-11750-1 23 Feb 2022

This script is in the tests folder. The tests folder is not controlled in the way the Dialin files are controlled so I might go back and forth in between different tests. This is not affecting any of the Dialin features. Last time it was used to program the HD RTC clock.

UI-DEN-10205-1 21 Dec 2021

Thanks.

UI-DEN-10205-1 21 Dec 2021

Out of curiosity, could you explain the difference between what #include go into the header file to those that are #include in the source file?

Edit: I understand that BluetoothInterface.h is needed, but what about the others?

DG-DEN-13834-1 18 Oct 2022

RESOLVED in CODE WALKTHROUGH.

DIALIN-DEN-11750-1 23 Feb 2022

For files in the peter folder I understand. However, with the repo scripts in this folder are typically shared with cross-functional teams and are "forward facing."

HD-DEN-14175-1 07 Nov 2022

Remove blank line.

DIALIN-DEN-11750-1 23 Feb 2022

I made it like HD.

DIALIN-DEN-11750-1 23 Feb 2022

RESOLVED.

HD-DEN-11114-1 31 Dec 2021

Done.

HD-DEN-11114-1 31 Dec 2021

Fixed it.

UI-DEN-10205-1 21 Dec 2021

Why does not this function have an argument name?

UI-DEN-10205-1 21 Dec 2021

I understand that the friend classes are for dev testing, are we planning to disable them later during the release?

HD-DEN-11114-1 03 Jan 2022

Magic numbers on these new for loops.

HD-DEN-11114-1 30 Dec 2021

1 and 0 is not clear. Better to say FIFO send and reset.

HD-DEN-11114-1 30 Dec 2021

We should verify bufferLength is >= to cal section byte size before doing this memcpy - otherwise, we will be overwriting memory.

DG-DEN-11750-1 01 Mar 2022

Concentrate pumps are available, right?

DG-DEN-11750-1 25 Feb 2022

Dara, I keep seeing this change in every code review - Compatibility.h is deleted and Compatible.h is created. Why do we keep seeing this?

DG-DEN-11750-1 25 Feb 2022

Done.

DG-DEN-11750-1 26 Feb 2022

Done.

HD-DEN-11750-2 26 Feb 2022

Sean Nash please reply.

DG-DEN-11750-1 24 Feb 2022

I think this alarm should be not commented out.

DG-DEN-11750-1 25 Feb 2022

Magic #.

DG-DEN-11928-1 28 Feb 2022

You must set isThisFirstFill to FALSE somewhere. This may not be the right place anymore - but it looks ok to me - why deleted?

DG-DEN-11928-1 28 Feb 2022

If alarm is recoverable, we should go to paused state. If not, we should not delete this line of code so we can exit fill mode. Otherwise, we get stuck here forever.

HD-DEN-8030-1 25 Jun 2021

RESOLVED in CODE WALKTHROUGH.

DIALIN-DEN-11750-1 23 Feb 2022

Done.

DIALIN-DEN-7792-1 14 Apr 2021

RESOLVED.

HD-DEN-7395-1 13 Apr 2021

Should probably go to normal state on fail as well - otherwise you get stuck here in self-test state.

HD-DEN-8030-1 25 Jun 2021

RESOLVED in CODE WALKTHROUGH.

HD-DEN-11750-2 02 Mar 2022

I think we should use <> for library includes. And they should go on top.

HD-DEN-13834-1 17 Oct 2022

RESOLVED IN CODE WALKTHROUGH

DG-DEN-11928-1 01 Mar 2022

Done.

UI-DEN-10205-1 21 Dec 2021

RESOLVED

UI-DEN-10205-1 21 Dec 2021

is $PAIRED_DEVICE_INFO an environment variable?

UI-DEN-10602-1 30 Dec 2021

RESOLVED.