•  

Comment Results

Review Name Created Custom Fields Content
HD-DEN-759-1 24 Nov 2019

This was something I temporarily added to handle the problem of trying to access RM46 registers while running in MinGW where such an attempt would cause a memory access error and crash the test case. Jim from VC finally gave me a way to do this from inside VC so I don't have to add this code - so I've since removed it.

HD-DEN-759-1 25 Nov 2019

I considered that. The flow rate ultimately needs to be converted to an unsigned PWM duty cycle for pump speed and a direction signal to the motor controller. This seemed to be a more natural way to achieve these ends. Also, with this approach there is no ambiguity about which direction is being commanded.

HD-DEN-759-1 25 Nov 2019

Wouldn't be simpler to have flow rate to be signed and have positive to be one direction and negative in another?

HD-DEN-759-1 25 Nov 2019

RESOLVED IN CODE WALKTHROUGH

HD-DEN-759-1 25 Nov 2019

Done.

HD-DEN-759-1 25 Nov 2019

Done.

HD-DEN-759-1 25 Nov 2019

Done.

HD-DEN-759-1 25 Nov 2019

Done.

HD-DEN-759-1 25 Nov 2019

Done.

HD-DEN-759-1 25 Nov 2019

Done.

HD-DEN-759-1 25 Nov 2019

Macro is for when you have 2 unsigned 32-bit data values that you want logged with the triggered alarm.

UI-DEN-704-1 26 Nov 2019

it's eChlid_HD.
it has been defined in the header file as enum.
e : is for enum
Chl : is for Channel
id : is id
_HD : is HD device
So it's a message recieved from HD channel id whcih has been defined as 0x020 in header file enum

UI-DEN-704-1 26 Nov 2019

it has been defined in the header file as enum.
if the command or message Id which has been received is not known by the Message Handler then it is 'isUnknow'

UI-DEN-704-1 26 Nov 2019

it's eChlid_HD.
it has been defined in the header file as enum.
e : is for enum
Chl : is for Channel
id : is id
_UI : is UI device
So it's a message sent out from UI channel id which has been defined as 0x100 in header file enum

UI-DEN-704-1 26 Nov 2019

t's eChlid_HD.
it has been defined in the header file as enum.
e : is for enum
Chl : is for Channel
id : is id
_UI : is UI device
So it's a message sent out from UI channel id which has been defined as 0x100 in header file enum

UI-DEN-704-1 26 Nov 2019

Sorry for the Typo ...
Fixed...

UI-DEN-704-1 25 Nov 2019

eChild?

UI-DEN-704-1 26 Nov 2019

RESOLVED.

UI-DEN-704-1 25 Nov 2019

isUnknown?

UI-DEN-704-1 26 Nov 2019

RESOLVED.

DIALIN-1 11 Dec 2019

Indentation misaligned.

DIALIN-1 30 Dec 2019

Doesn't let me to click complete seem like I need to put a reply here.
So :
The interpretation of PEP8 were not correct and Leo suppose to fix it.

RESOLVED

UI-DEN-626-1 24 Oct 2019

initialize with nullptr for consistency

HD-DEN-759-1 26 Nov 2019

RESOLVED in CODE WALKTHROUGH.

HD-DEN-752-2 03 Dec 2019

1. Space between arithmetic signs;
2. Did you want to use the STUCK_BUTTON_TIMEOUT instead of 1000 if they are correlated?

HD-DEN-759-1 26 Nov 2019

Short for fault mode. Due to formatting, it was useful to keep all mode enums at same length - so abbreviated to four letters.

UI-DEN-704-1 05 Dec 2019

Can Interface has its own error information.
What I added here is an extra information.

HD-DEN-759-1 26 Nov 2019

RESOLVED in CODE WALKTHROUGH.

HD-DEN-752-2 04 Dec 2019

RESOLVED in CODE WALKTHROUGH.

DIALIN-1 27 Nov 2019

Done

DIALIN-1 26 Nov 2019

Yes, we will stop listening to the can interface. This is the destructor of the class. Once we are done, it is game over. Also made the thread a daemon so it will autoclose when the program exits.

DIALIN-1 26 Nov 2019

How are we handling negative values in case they are generated by mistake?

HD-DEN-759-1 04 Dec 2019

Add TODO comment.

DIALIN-1 13 Dec 2019

DECISION: will leave as is for now.

HD-DEN-759-1 03 Dec 2019

In general,
I would explicitly assign numbers to enums and also categorizing them and put a little gap between each category.

HD-DEN-759-1 03 Dec 2019

I would add a conversion factor equal to 1000.

DG-DEN-1125-1 11 Dec 2019

I see a change to 0xA000, but then another 0xA100. I assume this is normal messages starting at 0xA000 and dialin at 0xA100? I would recommend starting DG normal messages at something lower like 0x2000 and then DG dialin messages at 0xA000.

HD-DEN-759-1 04 Dec 2019

RESOLVED IN CODE WALKTHROUGH. THIS WOULD BE THE PREFERRED WAY OF DOING IF.

HD-DEN-431-1 10 Jan 2020

RESOLVED in CODE WALKTHROUGH.

HD-DEN-759-1 04 Dec 2019

Done. And yes, we do have MIN and MAX macros in common.h.

DIALIN-1 05 Dec 2019

There are so many hex values roaming around the code which has not been defined in one place to be able to refer to.
Please collect them in one place with a line of description.
Or even if it is referring to the "X:\Engineering\Denali\06- Software Design\Messaging\Message List.xlsx" (which I found some there) please make a note to refer to that one if group of them can be read in that excel sheet.

HD-DEN-759-1 05 Dec 2019

Spaces between arithmetic sign and arguments, also did you want to add spaces between (v, l, and u)? If so, we need to do it everywhere in this code and everywhere else.

DIALIN-1 26 Nov 2019

How are we handling negative values in case they are generated by mistake?

HD-DEN-759-1 10 Dec 2019

RESOLVED in CODE WALKTHROUGH.

UI-DEN-1090-1 19 Dec 2019

RESOLVED.

DIALIN-1 05 Dec 2019

These hex values were assigned by Sean. I have an idea why? He is making sure that the number used is the one bit move to the left. Done. Please resolve.

HD-DEN-431-1 30 Dec 2019

Local variables should be declared at top of scope.

DIALIN-1 11 Dec 2019

RESOLVED.

HD-DEN-1312-1 06 Jan 2020

What added functionality triggered the creation of the assembly and command files?

DIALIN-1 12 Dec 2019

RESOLVED.