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

Review Summary: No summary


----------------------------------------
File: firmware/App/Modes/ModeFill.c
Revision Comment by Sean Nash on 28 March 2023, 16:07 
https://devapps.diality.us/cru/DG-DEN-15014-1#c16938

  Doesn't look like this list is complete.  In my opinion, init
  functions should be allowed to generalize here (e.g. Fill mode
  variables initialized.).

  Reply by Dara Navaei on 28 March 2023, 16:15
  > Done

    Reply by Sean Nash on 28 March 2023, 16:50
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 28 March 2023, 16:08 
https://devapps.diality.us/cru/DG-DEN-15014-1#c16939

  Needs param so caller can set it to FALSE too?

  Reply by Dara Navaei on 28 March 2023, 16:42
  > Done

    Reply by Sean Nash on 28 March 2023, 16:52
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 28 March 2023, 16:10 
https://devapps.diality.us/cru/DG-DEN-15014-1#c16940

  Prefer to say RO only != TRUE so that any other value than 1
  will cause RR alarm to be enforced.

  Reply by Dara Navaei on 28 March 2023, 16:13
  > Done

    Reply by Sean Nash on 28 March 2023, 16:51
    > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: firmware/App/Modes/ModeFill.h
Revision Comment by Sean Nash on 28 March 2023, 16:01 
https://devapps.diality.us/cru/DG-DEN-15014-1#c16935

  Should take a BOOL param and set RO only flag to given BOOL.
  As it is, you can only set to TRUE - never FALSE.

  Reply by Sean Nash on 28 March 2023, 16:55
  > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: firmware/App/Services/SystemCommMessages.c
Revision Comment by Sean Nash on 28 March 2023, 15:57 
https://devapps.diality.us/cru/DG-DEN-15014-1#c16933

  This is really a BOOL (FALSE/TRUE).  Why not make result a BOOL
  instead of U32?

  Reply by Dara Navaei on 28 March 2023, 16:43
  > Changed it to TRUE/FALSE.

    Reply by Sean Nash on 28 March 2023, 16:56
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 28 March 2023, 16:00 
https://devapps.diality.us/cru/DG-DEN-15014-1#c16934

  Standby Solo too?  In case Dialin sends by proxy.

  Reply by Dara Navaei on 28 March 2023, 16:17
  > Done

    Reply by Sean Nash on 28 March 2023, 16:54
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 28 March 2023, 16:02 
https://devapps.diality.us/cru/DG-DEN-15014-1#c16936

  Need else for if FALSE - disable RO only.

  Reply by Dara Navaei on 28 March 2023, 16:43
  > Updated the code.

    Reply by Sean Nash on 28 March 2023, 16:54
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 28 March 2023, 16:04 
https://devapps.diality.us/cru/DG-DEN-15014-1#c16937

  I don't think we need to ack/nak since we always respond with
  response msg.

  Reply by Dara Navaei on 28 March 2023, 16:17
  > Done

    Reply by Sean Nash on 28 March 2023, 16:53
    > What I meant is that we don't need to send anything other
    > than response.

      Reply by Dara Navaei on 28 March 2023, 16:59
      > Check the diff of the last two commits.

        Reply by Sean Nash on 28 March 2023, 17:03
        > I did.  I still see a call to sendAckResponseMsg() and
        > I don't think it should be here.

          Reply by Dara Navaei on 28 March 2023, 17:07
          > Done

          Reply by Sean Nash on 28 March 2023, 17:08
          > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: MsgDefs.h
Revision Comment by Sean Nash on 28 March 2023, 15:53 
https://devapps.diality.us/cru/DG-DEN-15014-1#c16932

  This message is sent by UI to DG.  Should be
  MSG_ID_UI_SET_DG_RO_ONLY_MODE.

  Reply by Dara Navaei on 28 March 2023, 16:47
  > Done

    Reply by Sean Nash on 28 March 2023, 16:50
    > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: firmware/App/Controllers/ROPump.c
Revision Comment by Sean Nash on 28 March 2023, 16:57 
https://devapps.diality.us/cru/DG-DEN-15014-1#c16959

  Remove TODO?

  Reply by Dara Navaei on 28 March 2023, 16:58
  > Done

    Reply by Sean Nash on 28 March 2023, 17:00
    > RESOLVED in CODE WALKTHROUGH.


---

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

Title: DG-DEN-15014_DG HD Dev HD DG Dvt Update Part 11
Statement of Objectives:


State: Closed

Summary: 

Author: Dara Navaei
Moderator: Dara Navaei
Reviewers: (5 active, 1 completed*)
     Sean Nash   (*)
     Michael Garthwaite
     wbracken
     Darren Cox
     jtaylor
     Steve Jarpe