This is a list of all comments for DG-DEN-13989-2.

Review Summary: No summary


----------------------------------------
File: firmware/App/Controllers/ConcentratePumps.c
Revision Comment by Dara Navaei on 24 April 2023, 15:19 
https://devapps.diality.us/cru/DG-DEN-13989-2#c17234

  What does MAG stand for?

  Reply by jtaylor on 24 April 2023, 15:30
  > Magnitude. Error limit for the case when the value is
  > reaching the resolution of the measurement, where the error
  > is specified as +-REL%, or +-MAG, whichever is greater.

Revision Comment by Dara Navaei on 24 April 2023, 14:07 
https://devapps.diality.us/cru/DG-DEN-13989-2#c17232

  Why are we resetting the persistent alarms here?

  Reply by jtaylor on 24 April 2023, 15:27
  > Two V&V runs seemed to trigger the pump speed alarm when pump
  > was slowing, then turned off.  It seemed possible that the
  > change in pump target speed was occurring while the
  > persistent alarm was on it's way to being triggered.

    Reply by Sean Nash on 24 April 2023, 17:08
    > I think we are regularly setting new targets on the
    > concentrate pumps when we are mixing (trying to keep mix
    > ratio same while RO flow is potentially changing due to
    > control).  That would mean the persistence is being cleared
    > all the time while mixing, essentially rendering the alarm
    > disabled.  I don't think we should reset here.

      Reply by jtaylor on 25 April 2023, 08:39
      > Removed.  We'll want to watch for continued pump speed
      > errors occurring on going to zero.  There seem to be both
      > RPM resolution and lag errors in the speed measurement,
      > causing problems with the alarm.  If problems continue,
      > we should consider increasing the trigger interval,
      > decreasing the clear interval, and/or resetting the alarm
      > when the target is set to zero (or taking a large step).

Revision Comment by wbracken on 11 April 2023, 14:14 
https://devapps.diality.us/cru/DG-DEN-13989-2#c17074

  0.0F

  Reply by jtaylor on 12 April 2023, 12:41
  > Thank you, done.

    Reply by wbracken on 12 April 2023, 17:21
    > RESOLVED IN CODE WALKTHROUGH

Revision Comment by wbracken on 18 April 2023, 14:26 
https://devapps.diality.us/cru/DG-DEN-13989-2#c17150

  Align "="

  Reply by jtaylor on 19 April 2023, 12:15
  > Corrected.

    Reply by wbracken on 24 April 2023, 10:48
    > RESOLVED IN CODE WALKTHROUGH

Revision Comment by Dara Navaei on 24 April 2023, 15:14 
https://devapps.diality.us/cru/DG-DEN-13989-2#c17233

  Does target speed being less than 3.0 mean the target speed is
  0.0? I understand that 3.0 mL/min is very close to 0.0 mL/min
  for this pump but I think we should be less than NEARLY_ZERO.

  Reply by jtaylor on 24 April 2023, 15:34
  > Yes. The pump minimum target is 3, attempting to set smaller
  > than that limit results in the pump being turned off.
  > 
  > The underlying issue is that stepConcentratePumpToTargetSpeed
  > does not initialize speedIncrease or hasTgtBeenReached, or
  > update the currentPumpSpeed target unless the
  > currentToTargetDiff is < nearly zero. Since the speeds are
  > quantized, we'll see zero more frequently than expected.
  > Particularly when stopping the pump, the Diff will be zero,
  > and currentPumpSpeed may never be updated, leading to odd
  > transient conditions.
  > 
  > By increasing the "zero" level, and adding the initialization
  > and else clauses for zero error and stopping the pump, we
  > guarantee that the speed setting is always set intentionally.
  > Also we already have a minimum speed parameter, used to
  > determine when to stop the motor, and attempts at motor
  > control at these low levels.

Revision Comment by wbracken on 11 April 2023, 14:15 
https://devapps.diality.us/cru/DG-DEN-13989-2#c17075

  0.0F, maybe a #define?

  Reply by jtaylor on 12 April 2023, 12:41
  > Thank you, done.

    Reply by wbracken on 12 April 2023, 17:26
    > RESOLVED IN CODE WALKTHROUGH


---

ID: DG-DEN-13989-2 https://devapps.diality.us/cru/DG-DEN-13989-2

Title: DG-DEN-13989_Concentrate Pump Cp1 Speed Error Observed
Statement of Objectives:


State: Closed

Summary: 

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