•  

Comment Results

Review Name Created Custom Fields Content
HD-DEN-11980-1 17 Feb 2022

RESOLVED in CODE WALKTHROUGH.

UI-DEN-10602-1 29 Nov 2021

Why commented out?

HD-DEN-11980-1 17 Feb 2022

RESOLVED in CODE WALKTHROUGH.

HD-DEN-11114-1 29 Nov 2021

This is a normal system message - should not be down here in the Dialin "test" message functions area.

HD-DEN-11980-1 17 Feb 2022

RESOLVED in CODE WALKTHROUGH.

HD-DEN-11980-1 17 Feb 2022

RESOLVED in CODE WALKTHROUGH.

HD-DEN-11098-1 02 Dec 2021

Updated this comment to
"Currently, we use 50.0 mL for testing. Remove this comment and replace 50.0 with 10.0 before merge to develop branch"

UI-DEN-10205-1 29 Nov 2021

Many of these methods do not have headers.

DG-DEN-11750-1 22 Feb 2022

Most of the files are VectorCAST that has been brought back from Code Clinic. Look at the files but they are auto generated so they are mostly passing through.

HD-DEN-11098-1 13 Dec 2021

Done

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.

DIALIN-DEN-10602-2 15 Dec 2021

These field numbers are off now. Start and End field numbers should be the same for each parameter parsed.

DIALIN-DEN-11750-1 22 Feb 2022

self.measured_raw_flow_rate_my_little_pony to self.measured_raw_flow_rate_lpm.

Otherwise publishing will not work.

HD-DEN-11114-1 31 Dec 2021

Done.

HD-DEN-11114-1 03 Jan 2022

Done.

HD-DEN-11114-1 03 Jan 2022

Done.

DIALIN-DEN-11750-1 23 Feb 2022

RESOLVED.

HD-DEN-11114-1 03 Jan 2022

No I do not think so. The CRCs are all checked in the self test.

UI-DEN-10205-1 21 Dec 2021

Do you still need this case?

HD-DEN-11098-1 30 Dec 2021

Recommend moving this rate to be with the others above.

HD-DEN-11098-1 13 Dec 2021

Recommend just setting up pumps and valves in this function rather than calling another function.

HD-DEN-11114-1 30 Dec 2021

Update message list with latest broadcast msg data details.

HD-DEN-11114-1 30 Dec 2021

Create and trigger new alarm.

HD-DEN-11114-1 30 Dec 2021

Please update message list details for this broadcast msg.

DIALIN-DEN-11750-1 23 Feb 2022

RESOLVED.

DIALIN-DEN-11750-1 22 Feb 2022

Shouldn't the dictionary format for HD/DG sensors be the same? Do we need dictionaries within the dictionary?

DIALIN-DEN-11750-1 22 Feb 2022

Please remove comments if they are no longer applicable to the test.

DG-DEN-11750-1 25 Feb 2022

Yes because the staging is branched form master that is very old. At the end of this code review I will update the master branch of common.

DG-DEN-11750-1 25 Feb 2022

You cannot run a function there. Also, the init function is the proper place to initialize the value.

DG-DEN-11750-1 25 Feb 2022

The alarm is not commented out currently, this might be due to an old commit.

DG-DEN-11928-1 01 Mar 2022

According Blain's PowerPoint, for bicard, the RO Pump speed is 400mL/min. 800mL/min is for acid.

HD-DEN-11750-2 02 Mar 2022

Yes, I tried a release build and it compiled.

UI-DEN-12121-1 03 Mar 2022

RESOLVED.

DG-DEN-11750-1 03 Mar 2022

RESOVLED IN CODE WALKTHROUGH.

HD-DEN-11750-2 03 Mar 2022

RESOVLED IN CODE WALKTHROUGH.

DG-DEN-11928-1 28 Feb 2022

Is this equation correct? Doesn't look right to me.

DG-DEN-11928-1 22 Mar 2022

Added doxygen comment

DG_HANDLE_BAD_FILL_STATE_START = 0, ///< Bad fill start state

DG-DEN-11928-1 28 Feb 2022

Do we still need temperature check here?

DG-DEN-11928-1 30 Mar 2022

Add test functions banner separator here.

DG-DEN-13460-2 22 Sep 2022

RESOLVED IN CODE WALKTHROUGH

UI-DEN-11980-1 16 Feb 2022

Fixed. Thanks!

DG-DEN-8030-1 17 Jun 2021

CONC_PUMPS_REVERSE_SPEED_ML_PER_MIN = -30.0 to indicate that the direction is in reverse.

DG-DEN-8030-1 17 Jun 2021

RESOLVED in CODE WALKTHROUGH.

DG-DEN-5963-1 13 Apr 2021

Nice catch. Done.

HD-DEN-11098-1 30 Dec 2021

Make 0.5 a #define.

DG-DEN-8030-1 17 Jun 2021

RESOLVED in CODE WALKTHROUGH.

DIALIN-DEN-11980-1 17 Feb 2022

Totally agree,
I just wanted to make sure we've planned to do that.
RESOLVED.

DG-DEN-8030-1 21 Jun 2021

RESOLVED in CODE WALKTHROUGH.

UI-DEN-7044-1 29 Mar 2021

This is an exception that you have a view which you don't have access to its object but in general:
whoever wants to use it to connect to a signal of another class should connect to that signal in its own scop/class and do it with its private slots.
So in a case like this, these connections should be moved to the interface.
But as I said it is an exception and there no better way than what has been done here.

DG-DEN-13834-1 14 Oct 2022

I had added comments on top of the declaration of the functions but I removed the schedule runs functions.