•  

Comment Results

Review Name Created Custom Fields Content
UI-DEN-4438-1 25 Aug 2020

OK, remove extra line between the } and respond to all my other indentation comments with the same comment for me to resolve.

DG-DEN-3421-1 10 Aug 2020

RPM is not the same as DAC. You are initializing the DAC to a minimum RPM (300 I think). You are also setting your minimum and maximum DAC values to RPMs.

UI-DEN-4438-1 20 Aug 2020

1. Why is indentation of } off?
2. Also no extra line between }

DG-DEN-4217-1 11 Aug 2020

Same comment as above.

DG-DEN-3421-1 11 Aug 2020

Aren't these pumps already set?

DG-DEN-3421-1 12 Aug 2020

Agreed. Will work on it once I started working on the story

DG-DEN-3421-1 12 Aug 2020

Target with capital "T"

DG-DEN-3421-1 12 Aug 2020

Agreed. Removed fabs

DIALIN-DEN-4344-1 21 Aug 2020

Why is this empty?

DG-DEN-3421-1 12 Aug 2020

Agreed. Will bring it back when I started working on this story

UI-DEN-3605-4 28 Aug 2020

RESOLVED.

DG-DEN-4217-1 12 Aug 2020

This module not yet converted to new Doxygen format.

DG-DEN-4217-1 12 Aug 2020

Done

DG-DEN-3421-1 12 Aug 2020

I see it above.

UI-DEN-3605-4 28 Aug 2020

RESOLVED.

DG-DEN-3421-1 11 Aug 2020

Where is target temp in this?

UI-DEN-4438-1 25 Aug 2020

Absolutely,
I'm doing it incrementally and slowly to not to make the code reviews harder and confusing git.
I did for so many and this file will get its turn soon.

DG-DEN-3421-1 13 Aug 2020

RESOLVED in CODE WALKTHROUGH.

UI-DEN-3605-4 28 Aug 2020

RESOLVED.

DG-DEN-3421-1 13 Aug 2020

RESOLVED in CODE WALKTHROUGH.

DG-DEN-3421-1 10 Aug 2020

PWM should not be set to flow rate. Use your macro to convert flow to PWM.

DG-DEN-3421-1 13 Aug 2020

RESOLVED in CODE WALKTHROUGH.

DG-DEN-3421-1 10 Aug 2020

Should param be F32?

DG-DEN-4217-1 12 Aug 2020

Remove period at the end to make it consistent with the rest.

UI-DEN-3605-4 28 Aug 2020

RESOLVED.

UI-DEN-4438-1 25 Aug 2020

done

UI-DEN-3605-4 25 Aug 2020

Remove extra line.

UI-DEN-4438-1 25 Aug 2020

Remove extra line.

DIALIN-DEN-3875-1 25 Aug 2020

These need to be filled in so the API documentation is populated

UI-DEN-3605-4 28 Aug 2020

RESOLVED.

HD-DEN-4308-3 29 Aug 2020

Done

HD-DEN-4308-3 29 Aug 2020

Done

UI-DEN-3253-1 11 Jun 2020

Done

DG-DEN-4217-1 12 Aug 2020

No more DMA needed?

DG-DEN-4217-1 12 Aug 2020

Doxygen /*@}/ missing at eof.

DG-DEN-3922-1 23 Jul 2020

RESOLVED in CODE WALKTHROUGH.

UI-DEN-3253-1 26 Jun 2020

Please align with the rest.

DG-DEN-4217-1 12 Aug 2020

Remove commented section if not used, otherwise add TODO comment.

UI-DEN-3253-1 15 Jun 2020

This is a placeholder animation until the alarms UI - HD integration work can begin. It is necessary for SW to meet the requirements until HD firmware has been updated.

UI-DEN-3149-1 15 Jun 2020

Semicolon

DG-DEN-3922-1 23 Jul 2020

RESOLVED in CODE WALKTHROUGH.

UI-DEN-3605-4 28 Aug 2020

RESOLVED.

DIALIN-DEN-4344-1 21 Aug 2020

It is so during the import process, the modules in the common directory are found. It's used to mark directories as python package directories.
If you remove it, then python will no longer find submodules.

https://docs.python.org/3/reference/import.html#regular-packages

If we add the following to _init.py then the user doesn't have to know the exact file location of a particular class:

So in dialin.
init_.py, we could add:
from hd.hemodialysis_device.py import HD

Then, a dialin script could do:

from dialin import HD

instead of

from dialin.hd.hemodialysis_device import HD


It abstracts away the file location so if we can rename files without the API user being affected.

Your comment reminded me that we weren't doing this yet and so I've added those changes to this review.

DG-DEN-3421-1 10 Aug 2020

Should be more than just closed loop here. Open loop included check for non-zero target speed. Can't do that here since zero is a valid target for delta pressure. Maybe define a NOT_SET value for delta pressure and check here to see if it's set to something other than NOT_SET.

HD-DEN-15306-3 02 Jun 2023

RESOLVED in CODE WALKTHROUGH.

DG-DEN-4169-1 31 Jul 2020

RESOLVED in CODE WALKTRHOUGH

DG-DEN-4169-1 31 Jul 2020

Do we want to have some kind of persistence for this?

UI-DEN-3875-1 20 Aug 2020

Right,
The coding style changed.
With the current monitors everyone using nowadays it's waste of space not to use extra available space.

DG-DEN-4169-1 03 Aug 2020

RESOLVED in CODE WALKTHROUGH.

UI-DEN-4964-1 29 Oct 2020

Pleae align semicolons.