•  

Comment Results

Review Name Created Custom Fields Content
HD-DEN-13598-2 27 Sep 2022

RESOLVED in CODE WALKTHROUGH.

DG-DEN-11928-1 23 Mar 2022

RESOLVED in CODE WALKTHROUGH.

DG-DEN-11928-1 31 Mar 2022

RESOLVED in WALKTHROUGH.

HD-DEN-14007-1 19 Dec 2022

Updated.

HD-DEN-12224-16 23 May 2022

We should keep this state until we are sure it is no longer needed.

DG-DEN-12224-7 23 May 2022

Done.

HD-DEN-12224-16 23 May 2022

RESOLVED in CODE WALKTHROUGH.

HD-DEN-12224-16 23 May 2022

RESOLVED in CODE WALKTHROUGH.

HD-DEN-12224-16 23 May 2022

Don't we still want to know if it's valid - and revert to defaults if not?

HD-DEN-12845-2 14 Jun 2022

RESOLVED in CODE WALKTHROUGH.

DIALIN-DEN-12224-1 20 Jun 2022

RESOLVED.

DG-DEN-12974-1 28 Jun 2022

This will be addressed in DEN-12931.

UI-DEN-12716-2 01 Jul 2022

Where would this value change?

DIALIN-DEN-12716-1 01 Jul 2022

Why are there 3 of these?

HD-DEN-12847-1 01 Jul 2022

Function re-factor makes this issue moot.

HD-DEN-12847-1 30 Jun 2022

I think we can handle both reservoirs with same code (no need for else). Probably just need to add a macro somewhere that converts a reservoir ID and primary/backup selection into a loadcell ID.

HD-DEN-12847-1 30 Jun 2022

I think this alarm should be an HD fault for now (no recovery).

HD-DEN-12847-1 30 Jun 2022

Do an explicit condition here. i.e. if ( TRUE == reservoirFull )

HD-DEN-12847-1 30 Jun 2022

Use TRUE (not true).

HD-DEN-12847-1 01 Jul 2022

RESOLVED in CODE WALKTHROUGH.

HD-DEN-12847-1 30 Jun 2022

General comment: Should floating point values be of the format N.NF? What is the coding standard? My experience has been that it's best to always specify the "F".

HD-DEN-12847-1 13 Jul 2022

RESOLVED in CODE WALKTHROUGH.

DG-DEN-12931-1 23 Jul 2022

No point in initializing here - gets set by exec function.

HD-DEN-12931-2 18 Jul 2022

SW_CONFIG_ENABLE_VALUE should be first in the conditional.

DG-DEN-12974-1 06 Jul 2022

That would also require a change to the requirements.

DG-DEN-12931-1 23 Jul 2022

Should 0.5 be a #define?

HD-DEN-12931-2 21 Jul 2022

F has been added.

HD-DEN-12931-2 21 Jul 2022

If no actions handled in this mode, add a comment saying so.

DG-DEN-12931-1 23 Jul 2022

If these comment outs are not permanent, add a TODO. Otherwise remove the commented out code.

DG-DEN-12931-1 22 Jul 2022

This blank line should stay - style is to have blank lines between declarations and code.

HD-DEN-12609-2 14 Jun 2022

This appears to be a timer counter (task intervals, not ms).

DG-DEN-12931-1 22 Jul 2022

Blank line between declarations and code.

DG-DEN-12931-1 23 Jul 2022

Fix alignment.

HD-DEN-12931-2 18 Jul 2022

Check NEARLY_ZERO #define. Value should be 0.00000001F. I believe previous review commented indicated that all floating point #defines should have "F" at the end.

DG-DEN-12931-1 03 Aug 2022

Removed the banner.

DG-DEN-12931-1 03 Aug 2022

Done.

HD-DEN-12931-2 22 Jul 2022

Why removed?

DG-DEN-12931-1 23 Jul 2022

DVT software configuration?

DG-DEN-12931-1 22 Jul 2022

Blank line between declarations and code.

HD-DEN-13141-6 21 Jul 2022

Why?

DG-DEN-8030-1 04 Jun 2021

Change order of comparison.

DG-DEN-8030-1 04 Jun 2021

Blank line after declaration.

UI-DEN-8252-1 02 Jun 2021

Note for reviewers:
I've created this review ahead of time
I am still working on the implementation

DG-DEN-7802-1 10 Jun 2021

Call checkDialysateConductivity() once at top of function. Then use return value for the two if conditions.
Also, this second if should be an else if to ensure you don't do both.

DG-DEN-8030-1 04 Jun 2021

I think we should add the new THd temperature sensor now. I know it's currently using TRo connection (and so we don't have a TRo), but we should handle that with build switch (THd reads from TRo channel and TRo can just be a copy of TDi for now if THD_USING_TRO_CONNECTOR build switch enabled, actual future code where both sensors exist on their own connectors if build switch is disabled).

DG-DEN-8030-1 04 Jun 2021

Is this needed. Should we just let Standby mode set actuators as it wants on entry?

DG-DEN-8030-1 04 Jun 2021

I think we should at least say that the mode variables are reset to re-start the mode or something.

DG-DEN-13598-2 26 Sep 2022

RESOLVED IN CODE WALKTHROUGH

DG-DEN-8030-1 04 Jun 2021

Why are some values aligned at left and others at right? I think all values should align at left.

DG-DEN-8030-1 04 Jun 2021

Not necessary - included in .h file.