This is a list of all comments for HD-DEN-16672-1.

Review Summary: No summary


----------------------------------------
File: firmware/App/Controllers/AirTrap.c
Revision Comment by Sean Nash on 20 February 2024, 12:23 
https://devapps.diality.us/cru/HD-DEN-16672-1#c19487

  Is this definition still needed?

  Reply by Sean Nash on 27 February 2024, 15:25
  > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 14 February 2024, 13:17 
https://devapps.diality.us/cru/HD-DEN-16672-1#c19462

  Mention periodic nature of this request.  Use "fill" verbage
  instead of "open up".

  Reply by Vinayakam Mani on 20 February 2024, 09:31
  > Done.

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

Revision Comment by Sean Nash on 14 February 2024, 13:17 
https://devapps.diality.us/cru/HD-DEN-16672-1#c19461

  Var is not really an input.  Just an output.

  Reply by Vinayakam Mani on 20 February 2024, 09:31
  > Done.

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

Revision Comment by Sean Nash on 14 February 2024, 13:19 
https://devapps.diality.us/cru/HD-DEN-16672-1#c19464

  Period is not really known by this module.  Don't call out 15
  minutes.  Just say when requested or something like that.

  Reply by Vinayakam Mani on 20 February 2024, 09:31
  > Done.

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

Revision Comment by Sean Nash on 23 February 2024, 09:02 
https://devapps.diality.us/cru/HD-DEN-16672-1#c19509

  Why is air pump on state checked here?  Seems redundant as it
  is checked again below.

  Reply by Vinayakam Mani on 26 February 2024, 10:08
  > Done.

    Reply by Sean Nash on 27 February 2024, 15:09
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 22 February 2024, 14:09 
https://devapps.diality.us/cru/HD-DEN-16672-1#c19507

  The lower level sensor doesn't need to be raw here.

  Reply by Sean Nash on 27 February 2024, 15:24
  > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 27 February 2024, 15:17 
https://devapps.diality.us/cru/HD-DEN-16672-1#c19512

  This looks backwards and nonsensical to me.  Does this work?

  Reply by Vinayakam Mani on 27 February 2024, 16:04
  > Since the utility is finding the difference of two numbers, I
  > see the logic is common.  for instance, On a positive case,
  > u32DiffWithWrap(9000, 26000) where 9000 is the adjustment
  > number and 26000 is the current timer value, the result is
  > 26000-9000 = 17000. on a negative case,
  > u32DiffWithWrap(9000,4000) where current timer value is 4000,
  > the result would be 0xFFFF FFFF - 9000 + 4000 + 1.  i.e: FFFF
  > FFFF -  5000 + 1.  Do you still see any concern?

    Reply by Sean Nash on 28 February 2024, 14:02
    > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: firmware/App/Controllers/AirTrap.h
Revision Comment by Sean Nash on 14 February 2024, 13:16 
https://devapps.diality.us/cru/HD-DEN-16672-1#c19460

  Rename to signalAirTrapPeriodicFill().

  Reply by Vinayakam Mani on 20 February 2024, 09:31
  > Done.

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


---

ID: HD-DEN-16672-1 https://devapps.diality.us/cru/HD-DEN-16672-1

Title: HD-DEN-16672_Air Trap Level Control Changes
Statement of Objectives:


State: Closed

Summary: 

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