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

Review Summary: No summary


----------------------------------------
File: firmware/App/Controllers/AirTrap.c
Revision Comment by Sean Nash on 15 February 2023, 10:05 
https://devapps.diality.us/cru/HD-DEN-14459-1#c16482

  Safer to zero this when you turn the pump on (could be done in
  one place in setAirPumpState function) - that way you don't
  have to worry about zeroing it everywhere you stop the pump.

  Reply by Michael Garthwaite on 15 February 2023, 10:09
  > This is the only position where its unassigned (changed due
  > to revert below). The counter itself is more controller level
  > than driver level as the persistence counter may be different
  > in other modes of operation. This counter is specifically
  > here to hold the air pump on 100ms after upper level is
  > detecting air. Should this check be moved to airpump.c?

    Reply by Sean Nash on 15 February 2023, 10:30
    > Are there any other ways to exit this state without zeroing
    > this counter (e.g. going to manual control in first if
    > above)?  Generally difficult to get all of the places where
    > exiting a state.  Safer to reset things on entry (i.e. line
    > 359 above).

      Reply by Michael Garthwaite on 15 February 2023, 10:45
      > Fixed. Thanks!

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

Revision Comment by Sean Nash on 15 February 2023, 10:07 
https://devapps.diality.us/cru/HD-DEN-14459-1#c16483

  Unlikely that we would go from Liq/Liq to Air/Air, but if we
  did, we would no longer turn the air pump off with this code
  removed.  Should we keep it?

  Reply by Michael Garthwaite on 15 February 2023, 10:10
  > it would take one state cycle ( 50ms ) to do that. The air
  > pump can't drive the fluid level that fast. We can keep it
  > for redundancy but the system would have much bigger problems
  > going on for that to happen.

    Reply by Michael Garthwaite on 15 February 2023, 10:21
    > Reverted. Thanks!

      Reply by Sean Nash on 15 February 2023, 10:28
      > RESOLVED in CODE WALKTHROUGH.


---

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

Title: HD-DEN-14459_Air Trap Fill Alarm Not Triggering
Statement of Objectives:


State: Closed

Summary: 

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