HD-DEN-14170

HD-DEN-14170_SW Dev Sprint 82 Darren
HD-DEN-14170_SW Dev Sprint 82 Darren
That was one the best explanations I have ever gotten about one of my confusions. I should have asked this a long time ago. Thanks a lot, Sean. RESOLVED in CODE WALKTHROUGH.

That was one the best explanations I have ever gotten about one of my confusions.
I should have asked this a long time ago.
Thanks a lot, Sean.

RESOLVED in CODE WALKTHROUGH.

RESOLVED in CODE WALKTHROUGH.

RESOLVED in CODE WALKTHROUGH.

Updated else only with rampSyringePump().

Updated else only with rampSyringePump().

It is not technically a boolean - it is a BOOL which is our implementation of a boolean (TRUE = 1, FALSE = 0). If we don't explicitly compare a BOOL to TRUE, compiler will consider any non-zero val...

It is not technically a boolean - it is a BOOL which is our implementation of a boolean (TRUE = 1, FALSE = 0).
If we don't explicitly compare a BOOL to TRUE, compiler will consider any non-zero value to be TRUE. So if memory is overwritten/corrupted to a random non-zero value, it would satisfy "if ( isSyringePumpPreLoaded() )" and s.w would do the wrong thing.
For extra safety, we prefer to compare to TRUE (1) or FALSE (0) depending on what would be safer.
For example, in this case we are deciding whether to seek plunger or alarm. We only seek plunger if the syringe pump was pre-loaded. It is safer to check pre-loaded flag is TRUE (1) to seek and ANY OTHER VALUE would alarm.
Also, when using the "==" compare operator, we will put the literal first. It looks awkward, but if we accidently use the "=" operator by mistake the compiler will give us a syntax error which saves us from having a tricky bug to fix later.

RESOLVED IN CODE WALKTHROUGH

RESOLVED IN CODE WALKTHROUGH

RESOLVED IN CODE WALKTHROUGH

RESOLVED IN CODE WALKTHROUGH

RESOLVED IN CODE WLAKTHROUGH

RESOLVED IN CODE WLAKTHROUGH

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.

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.

Per Sean - "No harm in adding "F" suffix, but not necessary for assignment to a float variable". Will leave as is for float vars.

Per Sean - "No harm in adding "F" suffix, but not necessary for assignment to a float variable". Will leave as is for float vars.

Per Sean - "No harm in adding "F" suffix, but not necessary for assignment to a float variable". Will leave as is for float vars.

Per Sean - "No harm in adding "F" suffix, but not necessary for assignment to a float variable". Will leave as is for float vars.

Per our discussion, this is the proper place to set the alarm. But will set stopPump = TRUE and add ELSE condition for move and check move conditions.

Per our discussion, this is the proper place to set the alarm. But will set stopPump = TRUE and add ELSE condition for move and check move conditions.

It is reset on Retract / Home completion. Once the Seek occurs, it is no longer valid so I reset it here on Seek completion.SelfTest.c uses this flag in progression from Retract to Preload to Seek ...

It is reset on Retract / Home completion. Once the Seek occurs, it is no longer valid so I reset it here on Seek completion.SelfTest.c uses this flag in progression from Retract to Preload to Seek to Prime. .

I attempted to do this earlier in the process, but getTreatmentParameterF32() caused a fault when called too early (treat params not ready?).. So I kept it here after determining it was before UI n...

I attempted to do this earlier in the process, but getTreatmentParameterF32() caused a fault when called too early (treat params not ready?).. So I kept it here after determining it was before UI needed it.

Thought we had decided that the standard would be to add the F?

Thought we had decided that the standard would be to add the F?

Ok to calculate here (this is called before UI needs it I assume). I was expecting this to be pre-calculated on exit of treatment params mode or on entry to pre-treatment mode when we have what we ...

Ok to calculate here (this is called before UI needs it I assume). I was expecting this to be pre-calculated on exit of treatment params mode or on entry to pre-treatment mode when we have what we need to do the calculation.

I would think this alarm would be triggered generally by driver because current mode said we shouldn't see syringe yet. Can we just set stopPump to TRUE here and nothing else?

I would think this alarm would be triggered generally by driver because current mode said we shouldn't see syringe yet.
Can we just set stopPump to TRUE here and nothing else?