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

Review Summary: No summary


----------------------------------------
File: firmware/App/Controllers/LoadCell.c
Revision Comment by Sean Nash on 06 January 2023, 16:02 
https://devapps.diality.us/cru/DG-DEN-13946-1#c15868

  Why change this to U08?

  Reply by wbracken on 09 January 2023, 11:09
  > Similar code is in other monitoring functions.
  > monitorTemperatureSensors, monitorPressureSensors iterate
  > over their list of sensors using U08.  monitorThermistors
  > iterates over thermistors using typedef
  > THERMISTORS_TEMP_SENSORS_T.  Changed load cells to use U08 to
  > be consistent with temperature and pressure monitoring.
  > 
  > Should probably change temperature, pressure and load cells
  > to use correct typedef.
  > 
  > Also, it looks like why the typedefs weren't used was in
  > order to set the initial value of the iterator to 0 (i.e, the
  > first sensor in the list). Seems like there should be a value
  > that is the first item in the list, i.e.
  > TEMPSENSORS_FIRST_SENSOR.

    Reply by Sean Nash on 10 January 2023, 08:26
    > Setting first iterator to 0 seems safer (in case somebody
    > adds a new enum before what used to be the first enum).  
    > Could also create a redundant 0 enum like below and use the
    > "first" enum to initialize a loop iterator safely:
    > enum list
    > \{
    >     FIRST_TEMP_SENSOR = 0,
    >     TPI_SENSOR = FIRST_TEMP_SENSOR,
    >     TPO_SENSOR,
    >     TD1_SENSOR,
    >     ....
    > \}
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Darren Cox on 13 January 2023, 10:40 
https://devapps.diality.us/cru/DG-DEN-13946-1#c15982

  No initial value for alarmID.  Can it end up not set before use
  in checkPersistentAlarm?

  Reply by wbracken on 13 January 2023, 11:31
  > Only used if the load cell is out of range.

    Reply by Darren Cox on 18 January 2023, 10:43
    > RESOLVED IN CODE WALKTHROUGH

----------------------------------------
File: firmware/App/Controllers/Thermistors.c
Revision Comment by Sean Nash on 09 January 2023, 10:15 
https://devapps.diality.us/cru/DG-DEN-13946-1#c15869

  Remove blank line.

  Reply by wbracken on 09 January 2023, 11:07
  > Removed.

    Reply by Sean Nash on 10 January 2023, 08:24
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Darren Cox on 13 January 2023, 10:33 
https://devapps.diality.us/cru/DG-DEN-13946-1#c15978

  Comment is 12bit specific, but code updated to use different
  references per thermistor.

  Reply by wbracken on 13 January 2023, 11:39
  > Corrected.

    Reply by Darren Cox on 17 January 2023, 12:00
    > RESOLVED IN CODE WALKTHROUGH

Revision Comment by Darren Cox on 13 January 2023, 10:35 
https://devapps.diality.us/cru/DG-DEN-13946-1#c15980

  Comment does not explain what the numbers are - 3, 10, 10.

  Reply by wbracken on 13 January 2023, 11:40
  > Updated comment

    Reply by Darren Cox on 17 January 2023, 15:32
    > RESOLVED IN CODE WALKTHROUGH

----------------------------------------
File: firmware/App/Controllers/TemperatureSensors.c
Revision Comment by Darren Cox on 13 January 2023, 10:38 
https://devapps.diality.us/cru/DG-DEN-13946-1#c15981

  Why remove initial value for alarmIndex?

  Reply by wbracken on 13 January 2023, 11:34
  > Only used if Temperature is in alarm.

    Reply by Darren Cox on 17 January 2023, 12:03
    > RESOLVED IN CODE WALKTHROUGH


---

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

Title: DG-DEN-13946_Certain Temperature Sensors Trigger Wrong Alarm
Statement of Objectives:


State: Closed

Summary: 

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