•  

Comment Results

Review Name Created Custom Fields Content
HD-DEN-6372-1 11 Feb 2021

The current design is for each mode to set actuators on entry to a new mode/sub-mode (e.g. this function) per that mode's initial state. Then, while in that mode/sub-mode, the mode's state machine will change actuators as required when moving from one state to another.

DG-DEN-6402-1 11 Feb 2021

These have been defined both in DG and HD before and now moved here as a common define.

DG-DEN-6402-1 11 Feb 2021

Yes, if drain pump is pumping too fast, the combined flow from drain pump and from RO pump is too much for the length of tubing after the two merge and go to drain. Starts to cavitate and causes a lot of turbulence.

DIALIN-DEN-5638-1 25 Jan 2021

The available log levels are listed in the docstring of the HD constructor.
It simplifies debugging someone's script to not have to import dialin logging levels when we need to debug their code.
Since the HD/DG logging is not intended to be used by dialin script writers, and is a dialin developer-only capability, adding enum imports to user scripts when we want to debug their code would be an unnecessary hindrance.

DIALIN-DEN-5980-1 12 Feb 2021

This function should be split up into separate functions that have a single responsibility

UI-DEN-4860-BLE-1 26 Jan 2021

RESOLVED

DIALIN-DEN-5638-1 26 Jan 2021

RESOLVED

DIALIN-DEN-5638-1 24 Jan 2021

Is it still required while you have the time entirely imported at line 20?

DIALIN-DEN-5638-1 22 Jan 2021

Why enum is not used?

DIALIN-DEN-5638-1 22 Jan 2021

Where this IP is used and what is the use case and what device is this IP refer to?

UI-DEN-5638-1 24 Jan 2021

How come this module doesn't have "HDSimulator = HDSimulator()" and still is using an instance of the HDSimulator?
it shows that :
1 - The name of the object has to be different than the class name itself so we will understand which one is used. (class or instance)
2 - If this one doesn't require defining the HDSimulator, then why it has been defined in the other ones.

DG-DEN-6402-1 11 Feb 2021

RESOLVED in CODE WALKTHROUGH.

HD-DEN-6372-1 11 Feb 2021

RESOLVED in CODE WALKTHROUGH.

HD-DEN-6402-1 08 Feb 2021

This is now included in the header file so we can remove here.

HD-DEN-6402-1 11 Feb 2021

Remove extra "/" from comment.

HD-DEN-6402-1 01 Feb 2021

Move execPrime() to end - only if no door open and no alarm stop signal.

DG-DEN-6402-1 11 Feb 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-6402-1 11 Feb 2021

RESOLVED in CODE WALKTHROUGH.

HD-DEN-6402-1 11 Feb 2021

Remove all extra "/" from all comments.

HD-DEN-6402-1 11 Feb 2021

RESOLVED in CODE WALKTHROUGH.

HD-DEN-6402-1 11 Feb 2021

Remove extra "/" from comment.

HD-DEN-6402-1 11 Feb 2021

RESOLVED in CODE WALKTHROUGH.

HD-DEN-6402-1 11 Feb 2021

Remove all extra "/" from all comments.

HD-DEN-6372-1 11 Feb 2021

RESOLVED in CODE WALKTHROUGH.

DIALIN-DEN-6078-1 25 Jan 2021

The valve_state is missing from the docstring

HD-DEN-6372-1 11 Feb 2021

Do we need to reset alarm signal flags?

HD-DEN-11250-1 16 Feb 2022

Reinstalled the case statement back to where it was.
case MSG_ID_HD_FAN_RPM_ALARM_START_TIME_OFFSET_OVERRIDE:
handleTestFansRPMAlarmStartTimeOffsetOverrideRequest( message );

HD-DEN-6402-1 11 Feb 2021

RESOLVED in CODE WALKTHROUGH.

UI-DEN-5830-2 19 Jan 2021

I will work on that later in prTx-UF story and it has only been fixed here since it wasn't passing the test sometimes while testing the Alarm design.

HD-DEN-6402-1 11 Feb 2021

RESOLVED in CODE WALKTHROUGH.

DG-DEN-8030-1 15 Jun 2021

Done.

UI-DEN-5638-1 25 Jan 2021

Also since the variable name stylings are not considered major issues, I will take a note and make sure to update the variable name at a later time.

HD-DEN-6402-1 11 Feb 2021

RESOLVED in CODE WALKTHROUGH.

HD-DEN-6200-1 21 Jan 2021

RESOLVED in CODE WALKTHROUGH.

HD-DEN-6402-1 11 Feb 2021

RESOLVED in CODE WALKTHROUGH.

UI-DEN-5638-1 25 Jan 2021


Please use the SquishQt recommended syntax and functions as has been explained and described in their examples.

In this change I am actually correcting a misuse of the squishqt api. waitForObjectExists will always return true after the first call without using utils.dict_update. This is because the text is omitted from the object that is being waited for. So, the object holding the text will always exist, and each successive call will not wait for the text to be updated. To pass, you were basically relying on the long delay added by cansend that occurs between each call to test_values. That delay happened to be longer than it takes for the text to be updated in the UI, allowing the tests to pass by accident. This kind of test design is fragile, as the test could fail at any time depending on how long cansend takes to send a message.


I would rather let SquishQt test and check the existence of the properties (in this case ".text") instead of another function adding a property to the dictionary without making sure that the object actually has the property and not checking the object tree.

(See above)

HD-DEN-6372-1 11 Feb 2021

RESOLVED in CODE WALKTHROUGH.

DG-DEN-8030-1 16 Jun 2021

Done.

DG-DEN-8030-1 17 Jun 2021

RESOLVED in CODE WALKTHROUGH.

UI-DEN-7135-1 09 Apr 2021

I'm not sure that QSettings does that for us.
But the design, for now, is that the settings are for reading only at boot up and no to write.
Later when we decided to have it save the settings will do the rest for the write/read protection.

HD-DEN-8103-1 25 Jun 2021

RESOLVED in CODE WALKTHROUGH.

UI-DEN-7135-1 09 Apr 2021

Removed.

UI-DEN-7135-1 10 Apr 2021

RESOLVED.

HD-DEN-9906-1 15 Nov 2021

Inputs: should be None
Outputs: Alarm may triggered

DIALIN-DEN-5751-1 25 Jan 2021

Also mentioned in the objectives:

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.
HD-DEN-9906-1 15 Nov 2021

Adjust the space before the equal signs.

DG-DEN-5963-1 14 Mar 2021

Shouldn't there be a few of these disinfect times? R1, R2, etc...

UI-DEN-5638-1 26 Jan 2021

It has been defined as a class not a variable after the name change.
So the name should be "Tx_States"

HD-DEN-6372-1 16 Feb 2021

Fixed.