Added but crucible is not showing this change. Please check bitbucket.

These variables are correctly initialized to the first enum in the list.

Not needed because the retry mechanism is going to be deleted in the next ticket.

I would rather keep it. Anyways, we need an else block after else if, as per MISRA guidelines.

I would rather keep it. Anyways, we need an else block after else if, as per MISRA guidelines.

Not needed because the retry mechanism is going to be deleted in the next ticket.

Same question as above: should we alarm if the retries failed?

This else is not needed because isFlashAddressValid is set to False at the top of the function (Optional to change).

This else is not needed because isFlashAddressValid is set to False at the top of the function (Optional to change).

If a retry goes wrong will you trigger an alarm?

Any fault alarm here?

Have these variables been initialized correctly?

Remove extra blank line.

We generally don't add { } to switch cases unless they require local variable declarations.

If we're not including a timestamp, can we just remove this line of code?

Why commented out?

Swap order of == for these 2 if statements - put constant on left side.

Why?

Why is UF volume being added here? If we add it, we need to change DD f/w too. But it is not clear to me why DD f/w needs to know the UF volume.

Use ternary operator to explicitly set to TRUE or FALSE.

Our TODO comments are all CAPs.

I think these if statements should be looking for valve state == VALVE_STATE_IDLE, not <

Fix implementation to return a BOOL (TRUE if SW_COMPATIBILITY_REV == ddVersion.compatibilityRev.

Should return T/F. Change comment to "TRUE if DD f/w is compatible with TD f/w, FALSE if not.".

Should return a BOOL (that is what ModeStandby is expecting).

Fix isDDCompatible() function so that it returns a T/F instead of a compatibility revision.