•  

Comment Results

Review Name Created Custom Fields Content
DIALIN-DEN-6372-1 16 Feb 2021

RESOLVED.

UI-DEN-6631-1 16 Feb 2021

RESOLVED

UI-DEN-7035-1 24 Mar 2021

Same as above

HD-DEN-9906-1 15 Nov 2021

Why there are TBDs in the comments?

HD-DEN-9906-1 15 Nov 2021

RESOLVED in CODE WALKTHROUGH.

HD-DEN-6372-1 17 Feb 2021

RESOLVED in CODE WALKTHROUGH.

DG-DEN-6402-1 17 Feb 2021

RESOLVED in CODE WALKTHROUGH.

HD-DEN-6402-1 17 Feb 2021

Why commented out?

DG-DEN-5980-1 21 Mar 2021

Dependencies are fine. It enables Bank7.

DG-DEN-6890-1 11 Mar 2021

RESOLVED in CODE WALKTHROUGH.

DG-DEN-5980-1 21 Mar 2021

In this test, I was NVDatMgmt to be tested but not anything else.

HD-DEN-7117-1 17 Mar 2021

Suggest combining into one line.

HD-DEN-7091-1 22 Mar 2021

Consider renaming this to sampleWaterResultEntered to clarify its purpose.

HD-DEN-7091-1 22 Mar 2021

Then I would rename the function/state to something less bubble detector and more used cartridge check 2.

HD-DEN-7091-1 22 Mar 2021

Fixed.

UI-DEN-7035-1 24 Mar 2021

These were created early on but they are no longer being used so I've now deleted them.

UI-DEN-7035-1 24 Mar 2021

Wouldn't be better to define just one set of common "settings..." variables and bind the others to that instead of having a chain of binding?
Preffered:

settings<1>: 85
settings<2>: 75

settingsBLE<1>:settings<1>
....
settingsWifi<1>: settings<1>
...

instead of the chain of:

settings<1>: 85
settings<2>: 75

settingsBLE<1>:settings<1>
....
settingsWifi<1>: settingsBLE<1>
...
UI-DEN-7035-1 24 Mar 2021

Do we need to set the background color?
I think the default ScreenItem backgroundColor would be fine!

UI-DEN-7035-1 24 Mar 2021

RESOLVED.

UI-DEN-7035-1 24 Mar 2021

The mail box has been removed from all message actually, as they are no longer needed. I've now removed it from the docstring here as well

DIALIN-DEN-5980-1 17 Feb 2021

Do these get/set functions still work? Do they need updating or are they obsolete?

DIALIN-DEN-5980-1 17 Feb 2021

Same comment as with DG accelerometer cal function.

DIALIN-DEN-7117-1 24 Mar 2021

These are defined in ui/hd_simulator.py as well. Common is probably the best place to put them. An enum would be good as Quang mentioned

HD-DEN-7347-1 02 Apr 2021

RESOLVED in CODE WALKTHROUGH.

HD-DEN-7117-1 01 Apr 2021

Replace 0x40 with constant and define it at beginning of *.c file.

DG-DEN-5963-1 03 Apr 2021

Done.

DG-DEN-5963-1 03 Apr 2021

I brought it back.

UI-DEN-7135-1 05 Apr 2021

I was trying to say the sorting may require to change later because the birthTime is not returning a valid date.
And maybe sorting on the name which always returns a valid value is better.
Also has a note that if it is going to change, be careful that it is going to use for different purposes other than only logging.

UI-DEN-7135-1 05 Apr 2021

I see, could you update the comment to say "other than only just logging" so it's clearer? I was totally confused by it

HD-DEN-7347-1 06 Apr 2021

Align comments.

DIALIN-DEN-7117-1 06 Apr 2021

Done.

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.

HD-DEN-11980-1 16 Feb 2022

Fixed. Thanks!

DIALIN-DEN-5751-1 01 Feb 2021

RESOLVED.

DIALIN-DEN-5751-1 29 Jan 2021

Replace let's with let us.

HD-DEN-12224-16 24 May 2022

Done.

HD-DEN-6402-1 11 Feb 2021

RESOLVED in CODE WALKTHROUGH.

HD-DEN-7395-1 08 Apr 2021

This is more of a request to run a self test by a mode (probably pre-treatment mode), so I think boolean is appropriate to indicate request was accepted.

HD-DEN-7395-1 08 Apr 2021

RESOLVED in CODE WALKTHROUGH.

DG-DEN-5963-1 14 Mar 2021

Delete blank line.

DG-DEN-5963-1 08 Apr 2021

...operating temperatures between 5 degrees C and 95 degrees C.

DIALIN-DEN-7135-1 11 Apr 2021

Removed.

UI-DEN-7135-1 05 Apr 2021

vGroup is being used so I think you can remove Q_UNUSED

UI-DEN-7135-1 05 Apr 2021

This function only returns 0 so it can be void

HD-DEN-7395-1 08 Apr 2021

Need to add state machine in this function. Call state handler functions from a switch statement.

DG-DEN-5963-1 14 Mar 2021

Why remove this check?

HD-DEN-6372-1 16 Feb 2021

Divide by 2 is averaging start and end rates of the ramp so that I can determine estimate of time needed to complete the ramp.

DIALIN-DEN-6631-1 15 Feb 2021

Remove extra lines.

DIALIN-DEN-6372-1 15 Feb 2021

Remove extra lines.

HD-DEN-6402-1 16 Feb 2021

RESOLVED in CODE WALKTHROUGH.