This is a list of all comments for DG-DEN-14706-2.

Review Summary: No summary


----------------------------------------
File: firmware/App/Services/SystemComm.c
Revision Comment by Sean Nash on 31 August 2023, 14:36 
https://devapps.diality.us/cru/DG-DEN-14706-2#c18794

  This should be inverse of what you had before.  I think this is
  too brief.

  Reply by Sean Nash on 01 September 2023, 11:19
  > RESOLVED in CODE WALKTHROUGH.

Revision Comment by jtaylor on 01 September 2023, 08:33 
https://devapps.diality.us/cru/DG-DEN-14706-2#c18799

  Is the "0" an ACK,NACK, PENDING_OVERRIDE or some other flag?
  Suggest a #define. (or a comment)

  Reply by Michael Garthwaite on 05 September 2023, 10:04
  > We typically dont #define for 0. However, the logic has been
  > reworked that we no longer need to compare by 0.

    Reply by jtaylor on 05 September 2023, 10:05
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 31 August 2023, 12:39 
https://devapps.diality.us/cru/DG-DEN-14706-2#c18784

  Already TRUE - why set again?
  I think "if" should be reversed - if not override, set used to
  FALSE.  And then after if, always set result to TRUE (means we
  found a match - which we did).

  Reply by Michael Garthwaite on 31 August 2023, 14:11
  > Pushed wip code. I'm unsure what or if we need to put
  > anything to compare override values.

    Reply by Sean Nash on 01 September 2023, 11:19
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 31 August 2023, 14:37 
https://devapps.diality.us/cru/DG-DEN-14706-2#c18795

  This if doesn't appear to be doing anything.  Remove.

  Reply by Sean Nash on 01 September 2023, 11:18
  > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 31 August 2023, 12:44 
https://devapps.diality.us/cru/DG-DEN-14706-2#c18785

  Remove extra blank line.

  Reply by Michael Garthwaite on 31 August 2023, 14:10
  > Fixed. Thanks!

    Reply by Sean Nash on 01 September 2023, 11:19
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 31 August 2023, 12:45 
https://devapps.diality.us/cru/DG-DEN-14706-2#c18786

  Too many () after &&?

  Reply by Michael Garthwaite on 31 August 2023, 14:11
  > Fixed. Thanks!

    Reply by Sean Nash on 01 September 2023, 11:19
    > RESOLVED in CODE WALKTHROUGH.


---

ID: DG-DEN-14706-2 https://devapps.diality.us/cru/DG-DEN-14706-2

Title: DG-DEN-14706_Need Mechanism For Alarm ID Can Message Not Acked And Alarm ID DG Can Message Not Acked 
Statement of Objectives:


State: Closed

Summary: 

Author: Michael Garthwaite
Moderator: Michael Garthwaite
Reviewers: (4 active, 3 completed*)
     Sean Nash   (*)
     wbracken   (*)
     jtaylor   (*)
     jpaguio
     Vinayakam Mani
     Dara Navaei
     Darren Cox