Controllers

Clone Tools
  • last updated a few minutes ago
Constraints
Constraints: committers
 
Constraints: files
Constraints: dates
Removed.

Removed.

I already removed the test code (at line 116). I just checked and it was removed in my local version. I already pushed DEN-14150. Please pull it gain to see if the test code is still there. Thank you.

I already removed the test code (at line 116).
I just checked and it was removed in my local version. I already pushed DEN-14150.
Please pull it gain to see if the test code is still there.
Thank you.

Why is this in utilities module? Doesn't it make more sense in fpga module?

Why is this in utilities module? Doesn't it make more sense in fpga module?

RESOLVED in CODE WALKTHROUGH.

RESOLVED in CODE WALKTHROUGH.

Remove blank line.

Remove blank line.

None of these structures appear to require packing - all 32-bit fields.

None of these structures appear to require packing - all 32-bit fields.

RESOLVED IN CODE WALKTHROUGH

RESOLVED IN CODE WALKTHROUGH

If this message is for DG CPLD only, should change enum to "MSG_ID_DG_CPLD_...".

If this message is for DG CPLD only, should change enum to "MSG_ID_DG_CPLD_...".

RESOLVED in CODE WALKTHROUGH.

RESOLVED in CODE WALKTHROUGH.

RESOLVED IN CODE WALKTHROUGH

RESOLVED IN CODE WALKTHROUGH

RESOLVED in CODE WALKTHROUGH.

RESOLVED in CODE WALKTHROUGH.

RESOLVED IN CODE WALKTHROUGH

RESOLVED IN CODE WALKTHROUGH

Remove test code.

Remove test code.

RESOLVED IN CODE WALKTHROUGH

RESOLVED IN CODE WALKTHROUGH

Remove test code.

Remove test code.

RESOLVED IN CODE WLAKTHROUGH

RESOLVED IN CODE WLAKTHROUGH

Looks like blank line is still there.

Looks like blank line is still there.

Done.

Done.

Done.

Done.

Done.

Done.

I know all the FW code is written in this style but it is always interesting to me, 1 - why don't just say if ( isSyringePumpPreLoaded() ) the function is a boolean why compare to TRUE to get a bo...

I know all the FW code is written in this style but it is always interesting to me,
1 - why don't just say

if ( isSyringePumpPreLoaded() )

the function is a boolean why compare to TRUE to get a bool expression?
if it was returning an int it made sense to have something like this:

if ( isSyringePumpPreLoaded() != 0 )

Which is still unnecessary in some cases.
2 - even if we want to use TRUE, why to put it at the beginning of the expression and don't put it at the end, like

if ( isSyringePumpPreLoaded() == TRUE )

So the first thing developer sees would be the important part which is the function not always a TRUE or FALSE.

RESOLVED IN CODE WALKTHROUGH

RESOLVED IN CODE WALKTHROUGH

If we want to do that for consistency, that's fine but let's handle that as a bulk update and going forward kind of thing rather than keep commenting individually on code reviews.

If we want to do that for consistency, that's fine but let's handle that as a bulk update and going forward kind of thing rather than keep commenting individually on code reviews.

RESOLVED in CODE WALKTHROUGH.

RESOLVED in CODE WALKTHROUGH.

RESOLVED in CODE WALKTHROUGH.

RESOLVED in CODE WALKTHROUGH.

RESOLVED in CODE WALKTHROUGH.

RESOLVED in CODE WALKTHROUGH.

I think the else can end right after ramp. Ok to do checks I would think.

I think the else can end right after ramp. Ok to do checks I would think.

RESOLVED in CODE WALKTHROUGH.

RESOLVED in CODE WALKTHROUGH.

RESOLVED in CODE WALKTHROUGH.

RESOLVED in CODE WALKTHROUGH.

Done.

Done.