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

Review Summary: No summary


----------------------------------------
File: firmware/App/Services/SystemCommMessages.c
Revision Comment by Sean Nash on 04 March 2022, 13:28 
https://devapps.diality.us/cru/DG-DEN-12121-1#c12366

  Looks like payload sent is not of this type, so why are we
  using it.  Should be BOOL?

  Reply by Michael Garthwaite on 09 March 2022, 12:19
  > Reworked function to give API control of HD communication
  > status.

    Reply by Sean Nash on 09 March 2022, 14:47
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 04 March 2022, 13:26 
https://devapps.diality.us/cru/DG-DEN-12121-1#c12365

  Remove blank line.

  Reply by Michael Garthwaite on 09 March 2022, 13:14
  > Fixed. Thanks!

    Reply by Sean Nash on 09 March 2022, 14:47
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 04 March 2022, 13:26 
https://devapps.diality.us/cru/DG-DEN-12121-1#c12364

  Add blank line after.

  Reply by Michael Garthwaite on 09 March 2022, 12:18
  > Fixed. Thanks!

    Reply by Sean Nash on 09 March 2022, 14:47
    > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: firmware/App/Services/SystemComm.c
Revision Comment by Dara Navaei on 06 March 2022, 20:47 
https://devapps.diality.us/cru/DG-DEN-12121-1#c12406

  I noticed you are using a U32 as a boolean without typecasting.
  I know a boolean is an unsigned integer behind the scenes but
  to be specific we should explicitly typecast.

  Reply by Michael Garthwaite on 09 March 2022, 14:33
  > Fixed. Thanks!

    Reply by Dara Navaei on 09 March 2022, 14:33
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Dara Navaei on 06 March 2022, 20:45 
https://devapps.diality.us/cru/DG-DEN-12121-1#c12405

  Why is this commented out?

  Reply by Michael Garthwaite on 07 March 2022, 14:46
  > Looks like it was commented out in this commit in August. 
  > 
  > http://dvm-linux02:7990/projects/DG/repos/dgfirmware/commits/47205a5002f27add91d8548f31c8a6fa18993fea#firmware/App/Services/SystemComm.c
  > 
  > Should I uncomment it?

    Reply by Sean Nash on 09 March 2022, 08:52
    > We need to decide what DG should do if HD comm times out.
    > Alarm may not make sense since HD apparently not connected
    > to receive/display alarm.
    > Appropriate action may be mode specific.

      Reply by Dara Navaei on 09 March 2022, 14:41
      > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 04 March 2022, 13:31 
https://devapps.diality.us/cru/DG-DEN-12121-1#c12368

  Add blank line between declarations and code.

  Reply by Michael Garthwaite on 09 March 2022, 12:18
  > Fixed. Thanks!

    Reply by Sean Nash on 09 March 2022, 14:46
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 04 March 2022, 13:31 
https://devapps.diality.us/cru/DG-DEN-12121-1#c12367

  Add space beetween if and (.

  Reply by Michael Garthwaite on 09 March 2022, 13:15
  > Fixed. Thanks!

    Reply by Sean Nash on 09 March 2022, 14:45
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 04 March 2022, 13:33 
https://devapps.diality.us/cru/DG-DEN-12121-1#c12371

  Do we always want to override status to TRUE?

  Reply by Michael Garthwaite on 07 March 2022, 14:50
  > Not Always. ovData will stay True until the Reset function is
  > called (which is then assigned ovInitData's value). Our
  > handler only calls the Set/Reset function. No parameters are
  > given.

    Reply by Michael Garthwaite on 08 March 2022, 10:09
    > Update. Function will be reworked to have proper override
    > control instead of always flipping to True.

      Reply by Sean Nash on 09 March 2022, 14:44
      > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 04 March 2022, 13:32 
https://devapps.diality.us/cru/DG-DEN-12121-1#c12369

  Add blank line between declarations and code.

  Reply by Michael Garthwaite on 09 March 2022, 12:18
  > Fixed. Thanks!

    Reply by Sean Nash on 10 March 2022, 14:42
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 04 March 2022, 13:32 
https://devapps.diality.us/cru/DG-DEN-12121-1#c12370

  Add space between if and (.

  Reply by Michael Garthwaite on 09 March 2022, 13:13
  > Fixed. Thanks!

    Reply by Sean Nash on 09 March 2022, 14:44
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Dara Navaei on 06 March 2022, 20:52 
https://devapps.diality.us/cru/DG-DEN-12121-1#c12407

  Add a space.

  Reply by Michael Garthwaite on 09 March 2022, 12:18
  > Fixed. Thanks!

    Reply by Dara Navaei on 09 March 2022, 14:32
    > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: firmware/App/Services/SystemComm.h
Revision Comment by Dara Navaei on 06 March 2022, 20:53 
https://devapps.diality.us/cru/DG-DEN-12121-1#c12409

  We do not have doxygen comments for the function declarations.

  Reply by Michael Garthwaite on 09 March 2022, 12:17
  > Fixed. Thanks!

    Reply by Dara Navaei on 09 March 2022, 14:32
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Dara Navaei on 06 March 2022, 20:52 
https://devapps.diality.us/cru/DG-DEN-12121-1#c12408

  Add a space.

  Reply by Michael Garthwaite on 09 March 2022, 12:17
  > Fixed. Thanks!

    Reply by Dara Navaei on 09 March 2022, 14:32
    > RESOLVED in CODE WALKTHROUGH.


---

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

Title: DG-DEN-12121_SW Dev Sprint 65
Statement of Objectives:


State: Closed

Summary: 

Author: Michael Garthwaite
Moderator: Michael Garthwaite
Reviewers: (1 active, 2 completed*)
     Sean Nash   (*)
     Dara Navaei   (*)
     hnguyen