•  

Comment Results

Review Name Created Custom Fields Content
DG-DEN-3504-1 12 Nov 2020

RESOLVED in CODE WALKTHROUGH.

DG-DEN-3421-2-1 13 Nov 2020

RESOLVED in CODE WALKTHROUGH.

DG-DEN-3421-2-1 13 Nov 2020

Done

DG-DEN-3421-2-1 13 Nov 2020

Done

DG-DEN-3421-2-1 13 Nov 2020

I removed the calculated and there is only the target.

UI-DEN-4691-1 12 Nov 2020

Is there a reason these shouldn't be deleted?

DG-DEN-3421-2-1 10 Nov 2020

There are two reactors in this array. This line only initializes the first. I would initialize all in a loop in init function and leave it uninitialized here.
Also, don't include the word override in the variable name.
Also, maybe this shouldn't be an override. And there is already a state in the status record, so maybe this should be removed.

DG-DEN-3504-1 17 Nov 2020

RESOLVED in CODE WALKTHROUGH.

DG-DEN-5864-1 30 Nov 2020

Why is the second parameter the same as the first one? Should we use function testSetTargetROPumpFlowAndPressure() instead?

DG-DEN-5846-1 30 Nov 2020

Should these monitor functions be called from top of exec? Why only in running state?

UI-DEN-5736-1 30 Nov 2020

Thanks for the comment.
I replied to the same comment.

DG-DEN-5846-1 30 Nov 2020

Are these new channels added to map in InternalADC.c?

DG-DEN-5846-1 30 Nov 2020

Alphabetize #includes.

DG-DEN-5846-1 30 Nov 2020

Add "_STATE" to end of each enum name.

DG-DEN-5846-1 30 Nov 2020

Name not needed in enum name. Consider dg_Thermistors instead.

DG-DEN-5846-1 30 Nov 2020

Our coding standard says constants s/b all caps with underlines (same as #defines).

DG-DEN-5846-1 30 Nov 2020

This function should probably be called at top of monitor function.

DG-DEN-5846-1 30 Nov 2020

Align second line.

DG-DEN-5846-1 30 Nov 2020

This assignment is always overwritten below. Leave state uninitialized here.

DG-DEN-3421-2-1 17 Nov 2020

Remove commented line or add TODO.

DG-DEN-5873-1 30 Nov 2020

Is this UT only code temporary? Adding a POST case with no functionality will cause POST to get stuck.

DG-DEN-5864-1 07 Dec 2020

Done

DG-DEN-5873-1 30 Nov 2020

Why not using persistent alarm here?

HD-DEN-4640-1 20 Oct 2020

Align all parameters and all corresponding Doxygen comments.

HD-DEN-4640-1 26 Oct 2020

RESOLVED in CODE WALKTHROUGH.

HD-DEN-4640-1 26 Oct 2020

Done.

DG-DEN-3421-1 11 Aug 2020

I assume this function is called by HD command handler. I think HD command to start a heat disinfection should probably be handled by Standby Mode and not here. Command is only valid in standby mode so it should take the command and decide if it's valid and request heat disinfect mode when it's ready (e.g. if it's performing a water sample it may defer starting heat disinfect until that's done).

HD-DEN-5674-2 30 Dec 2020

Fixed.

DG-DEN-3504-1 10 Nov 2020

Done.

HD-DEN-5674-2 04 Jan 2021

RESOLVED in CODE WALKTHROUGH.

DG-DEN-3504-1 10 Nov 2020

Rounded the conversion.

UI-DEN-7135-1 12 Apr 2021

RESOLVED

DG-DEN-3504-1 10 Nov 2020

Done.

DG-DEN-5963-1 13 Apr 2021

Done.

DG-DEN-3504-1 10 Nov 2020

Done.

HD-DEN-5674-2 30 Dec 2020

RESOLVED in CODE WALKTHROUGH.

DG-DEN-3504-1 10 Nov 2020

Done.

HD-DEN-5674-2 30 Dec 2020

we have (formal) instead of we've (informal)

HD-DEN-5674-2 30 Dec 2020

we are (formal) instead of we're (informal)

DG-DEN-3421-2-1 11 Nov 2020

Calculated is the value that is calculated using the equation. Target the value that is set to the fans, for instance if it is ramp up mode and the calculated value is greater than the limit, the target becomes the limit and not the calculated. Also, the calculated value is used to compare it with the newest value to decide whether it is in ramp up mode or ramp down. I renamed it to previous duty cycle for better clarity.

DG-DEN-3421-2-1 17 Nov 2020

RESOLVED in CODE WALKTHROUGH.

DIALIN-DEN-5328-1 10 Nov 2020

This code review contains all changes up to the end of sprint 31 not already included in http://dvm-linux02:8060/cru/DIALIN-DEVELOP-BUG-FIXES-1

DIALIN-DEN-11250-1 16 Feb 2022

Changed df_object to hd_object

DIALIN-DEVELOP-BUG-FIXES-1 11 Nov 2020

Done

HD-DEN-5674-2 30 Dec 2020

These comments are aligned when not commented out (switched off).

HD-DEN-13834-1 14 Oct 2022

Done.

UI-DEN-4964-1 03 Jan 2021

RESOLVED

DG-DEN-3421-2-1 11 Nov 2020

Yes. These are the internal temperature sensors. I added the word internal for better clarity.

DG-DEN-3421-2-1 13 Nov 2020

Consider moving to corresponding module (ModeHeatDisinfect).

UI-DEN-4964-1 03 Jan 2021

RESOLVED