•  

Comment Results

Review Name Created Custom Fields Content
HD-DEN-7395-1 13 Apr 2021

Should AND this bit off from register so we don't interfere with other bits that may have been set by other drivers (e.g. bubble detector).
Should be something like fpgaActuatorSetPoints.fpgaSensorTest &= ~FPGA_BLOOD_LEAK_SELF_TEST_CMD;

DD-LEAH-225-1 16 Oct 2024

Consider adding a FIRST heater to enum.

HD-DEN-11980-1 16 Feb 2022

Dara and I have found out that this code review for common is misconfigured. The source and destination branches are misconfigured. The branch to review should have been from DEN-11980_sw_dev_sprint_64 and the "branch from" should have been from staging. This code review has staging as the branch to review with master as the "branch review."

HD-DEN-8030-1 25 Jun 2021

Done

HD-DEN-8030-1 21 Jun 2021

Do we need to do something with the payload?

HD-DEN-8030-1 25 Jun 2021

RESOLVED in CODE WALKTHROUGH.

UI-DEN-11980-1 16 Feb 2022

RESOLVED.

UI-DEN-11980-1 16 Feb 2022

RESOLVED.

UI-DEN-11980-1 17 Feb 2022

Fixed. Thanks!

HD-DEN-11098-1 02 Dec 2021

RESOLVED in CODE WALKTHROUGH.

HD-DEN-11098-1 02 Dec 2021

RESOLVED in CODE WALKTHROUGH.

HD-DEN-11098-1 13 Dec 2021

Done.

HD-DEN-11098-1 13 Dec 2021

Done

HD-DEN-11098-1 13 Dec 2021

Done

DIALIN-DEN-11750-1 23 Feb 2022

RESOLVED.

UI-DEN-10602-1 21 Dec 2021

RESOLVED

DIALIN-DEN-11750-1 23 Feb 2022

The software configurations and calibration excel reports are protected. They are protected because the format of the reports do not have to change the scripts rely on them to read the data back and send them to the firmware. The only cells that are not protected are the cells that the users can changed the values (i.e. the calibration values). This feature is optional a report can be created that is not protected.

DIALIN-DEN-11750-1 23 Feb 2022

RESOLVED.

DIALIN-DEN-11750-1 22 Feb 2022

Why is this test commented out but not the HD?

HD-DEN-11114-1 03 Jan 2022

Done.

HD-DEN-11114-1 30 Dec 2021

.crc?

HD-DEN-11114-1 30 Dec 2021

Is CRC check on individual cal section done elsewhere or should it be considered here as well?

HD-DEN-11750-2 24 Feb 2022

Did we ever get clarification on these threshold values?

DG-DEN-11928-1 28 Feb 2022

Does this alarm need persistence? Looks like it used to have a timeout but now it's immediate.

DG-DEN-11750-1 25 Feb 2022

Done.

DG-DEN-11750-1 25 Feb 2022

Done.

DG-DEN-11750-1 25 Feb 2022

Done.

DG-DEN-11750-1 25 Feb 2022

The variable is initialized in the init().

HD-DEN-11750-2 26 Feb 2022

This code has been removed already.

HD-DEN-11750-2 26 Feb 2022

That is right.

DG-DEN-11928-1 28 Feb 2022

These are both minimums. Should be something like MIN_INPUT_WATER_TEMP_WARNING and MIN_INPUT_WATER_TEMP_ALARM.

DG-DEN-11750-1 02 Mar 2022

This will be addressed once we were working on deciding the number of rinses.

HD-DEN-11750-2 24 Feb 2022

Is the commented test code still needed?

DG-DEN-11928-1 28 Feb 2022

Do we still want to check temperature here? I thought this was no longer being done. Check with Dara.

DG-DEN-8030-1 16 Jun 2021

Done.

DG-DEN-8030-1 17 Jun 2021

RESOLVED in CODE WALKTHROUGH.

UI-DEN-11980-1 16 Feb 2022

Fixed. Thanks!

UI-DEN-11980-1 16 Feb 2022

RESOLVED.

DG-DEN-5963-1 14 Apr 2021

Yes, the purpose is to check whether the level changes, either going up or down.

HD-DEN-8030-1 25 Jun 2021

RESOLVED in CODE WALKTHROUGH.

DG-DEN-5963-1 14 Apr 2021

Done.

HD-DEN-8030-1 21 Jun 2021

Recommend breaking each state handler into a function.

DIALIN-DEN-7792-1 14 Apr 2021

Good catch, I've removed them now

HD-DEN-7395-1 13 Apr 2021

Same comment as above re: timeout.

HD-DEN-7395-1 13 Apr 2021

There is a SELF_TEST_STATUS_T enum (pass/fail) defined in fwcommon Common.h. Consider returning this type instead of BOOL.

DG-DEN-5963-1 13 Apr 2021

Done.

DIALIN-DEN-7792-1 14 Apr 2021

Mismatch filename.

DG-DEN-7802-1 26 Jun 2021

Fixed.

HD-DEN-11980-1 17 Feb 2022

RESOLVED in CODE WALKTHROUGH.

HD-DEN-11098-1 23 Nov 2021

You can remove this line here. That pump has not yet been run.