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

Review Summary: No summary


----------------------------------------
File: firmware/App/Modes/ModeFill.c
Revision Comment by Sean Nash on 22 February 2024, 10:39 
https://devapps.diality.us/cru/DG-DEN-16674-1#c19504

  Is this 5% definition still used?
  I don't think these are technically percentages (i.e. we do not
  mean 0.2%, we mean 20%).

  Reply by Dara Navaei on 22 February 2024, 10:51
  > Yes this is still used in the deliver state. I changed the
  > names in the #defines

    Reply by Sean Nash on 22 February 2024, 11:06
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 29 February 2024, 10:28 
https://devapps.diality.us/cru/DG-DEN-16674-1#c19528

  These are Dialin commands?  If so, prefix each enum with CMD_
  and clarify this in comment.

  Reply by Dara Navaei on 29 February 2024, 10:58
  > Done

    Reply by Sean Nash on 29 February 2024, 12:50
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 29 February 2024, 10:30 
https://devapps.diality.us/cru/DG-DEN-16674-1#c19529

  If these are Dialin commands, add a condition around this
  switch that Dialin has logged in.

  Reply by Dara Navaei on 29 February 2024, 10:53
  > Done

    Reply by Sean Nash on 29 February 2024, 12:49
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 27 February 2024, 15:54 
https://devapps.diality.us/cru/DG-DEN-16674-1#c19515

  Does this mean we aren't checking the CD1 sensor at all?

  Reply by Dara Navaei on 29 February 2024, 08:32
  > CD1 is used to check the difference between CD1 and CD2. CD1
  > is not used to check the reading of acid.

    Reply by Sean Nash on 29 February 2024, 10:31
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 29 February 2024, 10:31 
https://devapps.diality.us/cru/DG-DEN-16674-1#c19531

  If this is a Dialin command, make sure Dialin is logged in
  before acting on it (throughout this module).

  Reply by Dara Navaei on 29 February 2024, 10:52
  > Done

    Reply by Sean Nash on 29 February 2024, 12:49
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 29 February 2024, 10:32 
https://devapps.diality.us/cru/DG-DEN-16674-1#c19532

  Make sure Dialin is logged in before accepting command.

  Reply by Dara Navaei on 29 February 2024, 10:50
  > Done

    Reply by Sean Nash on 29 February 2024, 12:49
    > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: NVDataMgmt.c
Revision Comment by Sean Nash on 27 February 2024, 15:55 
https://devapps.diality.us/cru/DG-DEN-16674-1#c19516

  All of these for loops are not following our coding standards.
  Should have bracketed scope even if single line of code in
  loop.

  Reply by Dara Navaei on 29 February 2024, 08:27
  > Will address in DEN-16693.

    Reply by Sean Nash on 29 February 2024, 10:35
    > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: firmware/App/Services/SystemCommMessages.c
Revision Comment by Sean Nash on 29 February 2024, 10:34 
https://devapps.diality.us/cru/DG-DEN-16674-1#c19533

  Do this (request fill mode) in the testSetModeFillForCal()
  function above (and only if Dialin is logged in).

  Reply by Dara Navaei on 29 February 2024, 10:50
  > Done. Added a check in the function in the Mode Fill like the
  > rest of the modes and drivers.

    Reply by Sean Nash on 29 February 2024, 12:50
    > I don't see change.  Did you push?

      Reply by Dara Navaei on 29 February 2024, 12:51
      > No I will check the Dialin check in the function like the
      > rest of the commands from Dialin.

        Reply by Sean Nash on 29 February 2024, 12:59
        > But this comment also asked that you remove the
        > transition to fill mode here and add it to the fill
        > mode function called above.  It's still here.

          Reply by Dara Navaei on 29 February 2024, 13:04
          > Done

          Reply by Sean Nash on 29 February 2024, 13:44
          > RESOLVED in CODE WALKTHROUGH.


---

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

Title: DG-DEN-16674_Td2 Calibration Feature
Statement of Objectives:


State: Closed

Summary: 

Author: Dara Navaei
Moderator: Dara Navaei
Reviewers: (5 active, 1 completed*)
     Sean Nash   (*)
     pvedantam
     jpaguio
     Vinayakam Mani
     Michael Garthwaite
     Darren Cox