•  

Comment Results

Review Name Created Custom Fields Content
HD-DEN-7091-1 22 Mar 2021

If DG somehow gets ahead of HD here, I worry we will get stuck. Consider second condition with "getDGSubMode() >= DG_FILL_MODE_STATE_BICARB_PUMP_CHECK". Same for states below.

HD-DEN-7091-1 22 Mar 2021

When using build switches, convention is to put the normal code first. So use #ifndef and swap the statements below.

HD-DEN-7091-1 22 Mar 2021

Use set dialysate temp from treatment parameters for trimmer heater target temp. Add 2 degrees for primary heater target temp.

DG-DEN-5980-1 22 Mar 2021

What I meant was, do any of the prior init functions above need nv-data (and bank 7) to be initialized before they are run?

HD-DEN-7091-1 23 Mar 2021

RESOLVED in CODE WALKTHROUGH.

HD-DEN-5980-1 21 Mar 2021

Where is this getHDFlowSensorsCalibrationRecord function located? Is it part of fwcommon changes? If yes, please add to code review.

HD-DEN-5980-1 22 Mar 2021

It is a Diality board but it is treated as an eval board since there is nothing attached to it.

HD-DEN-5980-1 23 Mar 2021

Done.

DG-DEN-5980-1 23 Mar 2021

RESOLVED in CODE WALKTHROUGH.

HD-DEN-7091-1 23 Mar 2021

RESOLVED in CODE WALKTHROUGH.

HD-DEN-7091-1 22 Mar 2021

Should be 0.0.

HD-DEN-7091-1 22 Mar 2021

Probably should be fabs() here. And change 1 to 1.0.

HD-DEN-7091-1 23 Mar 2021

RESOLVED in CODE WALKTHROUGH.

HD-DEN-7091-1 22 Mar 2021

Add blank line here.
Also add comment for these if statements to describe the conditions. Looks like first if for when to end the test and second if is pass/fail criteria.

HD-DEN-7091-1 23 Mar 2021

RESOLVED in CODE WALKTHROUGH.

HD-DEN-7117-1 23 Mar 2021

Done.

DIALIN-DEN-5980-1 23 Mar 2021

I removed them.

DIALIN-DEN-5980-1 23 Mar 2021

It is the length of the received message. I renamed it.

DIALIN-DEN-5980-1 23 Mar 2021

I removed the function.

DIALIN-DEN-5980-1 23 Mar 2021

I split the function into smaller pieces.

HD-DEN-5980-1 24 Mar 2021

Done.

UI-DEN-7035-1 24 Mar 2021

Why onPressed?
Should be onClicked.

UI-DEN-7035-1 22 Mar 2021

No Msg Box?

HD-DEN-11980-1 16 Feb 2022

Fixed. Thanks!

DG-DEN-8030-1 15 Jun 2021

Done.

UI-DEN-5751-1 29 Jan 2021

Add function missing header.

UI-DEN-5751-1 29 Jan 2021

Add missing function header.

HD-DEN-6402-1 11 Feb 2021

RESOLVED in CODE WALKTHROUGH.

DIALIN-DEN-5751-1 01 Feb 2021

Please refer to the general comment I put,
These codes are not maintained by UI.


The only files that were involved in these changesets are:

dialin/ui/hd_simulator.py
dialin/common/msg_defs.py
dialin/squish/denaliMessages.py
And all the other files are merged from master and other branches which were merged into master.

DIALIN-DEN-5751-1 01 Feb 2021

Please refer to the general comment I put,
These codes are not maintained by UI.


The only files that were involved in these changesets are:

dialin/ui/hd_simulator.py
dialin/common/msg_defs.py
dialin/squish/denaliMessages.py
And all the other files are merged from master and other branches which were merged into master.

DIALIN-DEN-5751-1 29 Jan 2021

Why CD2 is commented out everywhere in this code?

DG-DEN-8030-1 16 Jun 2021

RESOLVED in CODE WALKTHROUGH.

HD-DEN-6402-1 11 Feb 2021

This is doxygen style comment if not in the same line.

HD-DEN-5887-1 11 Feb 2021

RESOLVED in CODE WALKTHROUGH.

DG-DEN-8030-1 16 Jun 2021

Good catch, thanks. I changed it.

HD-DEN-7395-1 08 Apr 2021

Addressed.

DG-DEN-5963-1 08 Apr 2021

Remove "for" at end of @param sentence.

HD-DEN-6402-1 16 Feb 2021

We are taking both actions on the same reservoir. Sean Nash can provide more details.

HD-DEN-6372-1 16 Feb 2021

Where?

HD-DEN-6372-1 16 Feb 2021

Done.

HD-DEN-6372-1 16 Feb 2021

Fixed.

HD-DEN-6372-1 16 Feb 2021

Is that a rule in our coding standard?

DIALIN-DEN-6372-1 15 Feb 2021

I would recommend using the following naming convention: hdPress instead of hdPres, which can be confused with HD present (hdPres).

DIALIN-DEN-6372-1 16 Feb 2021

RESOLVED.

DIALIN-DEN-6631-1 16 Feb 2021

RESOLVED

UI-DEN-6631-1 16 Feb 2021

RESOLVED

HD-DEN-7395-1 24 Mar 2021

These need to be called from switch statement at bottom of SystemComm.c.

DIALIN-DEN-5980-1 17 Feb 2021

Add comment for this.

DIALIN-DEN-5980-1 17 Feb 2021

Consider renaming this module/class to clarify these utility functions are for non-volatile memory support.

DIALIN-DEN-5980-1 17 Feb 2021

Comment to clarify purpose.