This is a list of all comments for DG-DEN-12847-1.

Review Summary: No summary


----------------------------------------
File: firmware/App/Controllers/LoadCell.c
Revision Comment by Sean Nash on 28 June 2022, 09:27 
https://devapps.diality.us/cru/DG-DEN-12847-1#c13177

  Are we sticking with this threshold?

  Reply by Darren Cox on 28 June 2022, 15:42
  > 10-20 seems like a better number, but my sample size is small
  > so I did not change it.

    Reply by Sean Nash on 01 July 2022, 10:34
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 28 June 2022, 09:06 
https://devapps.diality.us/cru/DG-DEN-12847-1#c13172

  Coding standard wants just one return statement in a function.
  Set a local boolean in if/else and return that.

  Reply by Sean Nash on 01 July 2022, 10:35
  > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 28 June 2022, 09:06 
https://devapps.diality.us/cru/DG-DEN-12847-1#c13173

  Remove blank line.

  Reply by Sean Nash on 01 July 2022, 10:34
  > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 28 June 2022, 09:23 
https://devapps.diality.us/cru/DG-DEN-12847-1#c13174

  Use our boolean #define (FALSE) instead of built-in (false).

  Reply by Darren Cox on 28 June 2022, 15:42
  > Updated

    Reply by Sean Nash on 01 July 2022, 10:34
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 28 June 2022, 09:24 
https://devapps.diality.us/cru/DG-DEN-12847-1#c13175

  Make this assignment explicitly BOOL (i.e. TRUE or FALSE), not
  implied bool.

  Reply by Darren Cox on 28 June 2022, 15:41
  > Updated

    Reply by Sean Nash on 01 July 2022, 10:34
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 28 June 2022, 09:25 
https://devapps.diality.us/cru/DG-DEN-12847-1#c13176

  Need to address TODOs.  Have we assessed what an appropriate
  max delta should be?  Alarm should be cleared if it is
  recoverable (not a fault) and not cleared immediately (see
  alarm properties).

  Reply by Darren Cox on 28 June 2022, 15:44
  > Alarm is not a fault. Updated clear immediate to false in
  > AlarmDefs.h in Staging.

    Reply by Sean Nash on 01 July 2022, 10:34
    > RESOLVED in CODE WALKTHROUGH.


---

ID: DG-DEN-12847-1 https://devapps.diality.us/cru/DG-DEN-12847-1

Title: DG-DEN-12847_SW Dev Sprint 71 Darren
Statement of Objectives:


State: Closed

Summary: 

Author: Darren Cox
Moderator: Darren Cox
Reviewers: (2 active, 3 completed*)
     Sean Nash   (*)
     Michael Garthwaite   (*)
     Dara Navaei   (*)
     wbracken
     Dong Nguyen