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

Review Summary: No summary


----------------------------------------
File: firmware/App/Controllers/Fans.c
Revision Comment by Sean Nash on 30 November 2020, 13:28 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6364

  Blank line between HALCOGEN #includes and others.  Alphabetize
  #includes in each set.

  Reply by Dara Navaei on 30 November 2020, 18:49
  > Done

    Reply by Sean Nash on 01 December 2020, 10:50
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by qnguyen on 30 November 2020, 13:47 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6383

  Use time interval rather than count.

  Reply by Dara Navaei on 30 November 2020, 18:49
  > You mean rename it?

    Reply by qnguyen on 01 December 2020, 08:56
    > Persistent alarm now uses time interval limit rather than
    > count. So we do not need to divide by the interval here.
    > This change eliminates the need to know the task interval
    > in which the checking function is getting called.

      Reply by Dara Navaei on 04 December 2020, 09:09
      > Done

        Reply by qnguyen on 07 December 2020, 10:45
        > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 30 November 2020, 13:30 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6368

  Add "_STATE" to end of each enum name.

  Reply by Dara Navaei on 30 November 2020, 18:52
  > Done

    Reply by Sean Nash on 01 December 2020, 10:49
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 30 November 2020, 13:29 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6365

  Add "states" to end of comment.

  Reply by Dara Navaei on 30 November 2020, 18:52
  > Done

    Reply by Sean Nash on 01 December 2020, 10:49
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 30 November 2020, 13:30 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6369

  Add "_STATE" to end of each enum name.

  Reply by Dara Navaei on 30 November 2020, 18:52
  > Done

    Reply by Sean Nash on 01 December 2020, 10:49
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by qnguyen on 30 November 2020, 13:55 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6392

  Mispelled Result?

  Reply by Dara Navaei on 30 November 2020, 18:53
  > Done

    Reply by qnguyen on 01 December 2020, 10:38
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 30 November 2020, 13:31 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6370

  Our coding standard says constants s/b all caps with underlines
  (same as #defines).

  Reply by Dara Navaei on 30 November 2020, 18:55
  > Done

    Reply by Sean Nash on 01 December 2020, 10:49
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 30 November 2020, 13:31 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6371

  Remove extra blank line.

  Reply by Dara Navaei on 30 November 2020, 18:55
  > Done

    Reply by Sean Nash on 01 December 2020, 10:49
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 30 November 2020, 13:35 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6375

  This assignment is always overwritten below.  Leave state
  uninitialized here.

  Reply by Dara Navaei on 30 November 2020, 18:56
  > Done

    Reply by Sean Nash on 01 December 2020, 10:47
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 30 November 2020, 13:34 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6374

  This part of test only makes sense for POST.  If we want to
  re-run this test before each treatment, we would fail this I
  think.

  Reply by Dara Navaei on 30 November 2020, 18:57
  > Yes, that is true. We never check for RPM being 0 after POST.

    Reply by Dara Navaei on 04 December 2020, 09:32
    > Removed part that was checking for 0 RPM.

      Reply by Sean Nash on 07 December 2020, 10:49
      > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 30 November 2020, 13:37 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6376

  Fix comment.  "They should be above...".

  Reply by Dara Navaei on 30 November 2020, 18:58
  > This comment is above the for loop. Are you saying it should
  > be above the if statement?

    Reply by Dara Navaei on 07 December 2020, 12:53
    > Fixed the sentence.

      Reply by Sean Nash on 09 December 2020, 11:44
      > RESOLVED in CODE WALKTHROUGH.

Revision Comment by qnguyen on 30 November 2020, 13:51 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6388

  Set result to pass before running the for loop. Remove failure
  count and set self test result to fail in place where we
  increase failure count.
  We can get rid of this if else.

  Reply by Dara Navaei on 30 November 2020, 19:01
  > Very good point. Done.

    Reply by qnguyen on 01 December 2020, 10:39
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 30 November 2020, 13:40 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6378

  This comment should fit on one line.

  Reply by Dara Navaei on 30 November 2020, 19:02
  > Done.

    Reply by Sean Nash on 01 December 2020, 10:47
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 30 November 2020, 13:42 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6379

  Why is this else (decreasing) not symmetric with increasing
  condition above?

  Reply by Dara Navaei on 30 November 2020, 19:03
  > The decrease is only 0.005, so we are never less that that
  > and I don't need to check for it.

    Reply by Dara Navaei on 04 December 2020, 09:21
    > Added the if statements to make it symmetrical.

      Reply by Sean Nash on 07 December 2020, 10:48
      > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 30 November 2020, 13:44 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6380

  Should these monitor functions be called from top of exec?  Why
  only in running state?

  Reply by Dara Navaei on 30 November 2020, 20:19
  > Yes. I put at the top of this function. Because there are
  > only two states and most of times we are running the fans. I
  > also did not want to monitor the fans every 50ms so I put it
  > here to monitor the fans whenever it is time to control the
  > fans.

    Reply by Dara Navaei on 04 December 2020, 09:13
    > Removed this function from here and moved it to execFans.

      Reply by Sean Nash on 07 December 2020, 10:47
      > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 30 November 2020, 13:46 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6382

  This function should probably be called at top of monitor
  function.

  Reply by Dara Navaei on 30 November 2020, 20:21
  > Moved it to the monitor function.

    Reply by Sean Nash on 01 December 2020, 10:45
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 30 November 2020, 14:12 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6399

  Why only a maximum speed?  I would think a minimum speed would
  be more important.

  Reply by Dara Navaei on 30 November 2020, 20:23
  > Added a check for minimum RPM assuming the RPMs should never
  > be zero.

    Reply by Sean Nash on 01 December 2020, 10:42
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 30 November 2020, 13:48 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6384

  100 should be 100.0.  If we use this in other places, maybe it
  can be a common #define.

  Reply by Dara Navaei on 30 November 2020, 20:25
  > I didn't realize we have this in common.h. I used that.

    Reply by Sean Nash on 01 December 2020, 10:44
    > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: firmware/App/Controllers/Heaters.c
Revision Comment by pmontazemi on 01 December 2020, 08:42 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6478

  Replace with TODO

  Reply by Dara Navaei on 01 December 2020, 09:23
  > Done

    Reply by pmontazemi on 01 December 2020, 10:55
    > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: firmware/App/Controllers/TemperatureSensors.c
Revision Comment by qnguyen on 30 November 2020, 13:45 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6381

  Revert this change. Persistent alarm uses time period instead
  of count.

  Reply by Dara Navaei on 30 November 2020, 20:30
  > Done

    Reply by qnguyen on 07 December 2020, 10:45
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 30 November 2020, 13:06 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6355

  Why is bit count declared like a floating point?

  Reply by Dara Navaei on 30 November 2020, 20:31
  > Done. I will cast them.

    Reply by Sean Nash on 01 December 2020, 10:52
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 30 November 2020, 13:07 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6356

  Why is temperature published so frequently?  Was 1 Hz really
  too slow?

  Reply by Dara Navaei on 30 November 2020, 20:32
  > I agree. It must have been from the past. Changed it to 1Hz.

    Reply by Sean Nash on 01 December 2020, 10:52
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 30 November 2020, 13:08 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6357

  What does this comment mean?

  Reply by Dara Navaei on 30 November 2020, 20:32
  > This showed in the merge. I will test them in the lab and
  > remove the comments.

    Reply by Sean Nash on 01 December 2020, 10:51
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by pmontazemi on 07 December 2020, 09:57 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6675

  align "(" on second line with the beginning of TEMPERATURE on
  right hand side of "=" sign, do the same with both lines 151
  and 153.

  Reply by Dara Navaei on 07 December 2020, 12:55
  > Done

    Reply by pmontazemi on 09 December 2020, 11:45
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 30 November 2020, 13:19 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6359

  Our coding standard says constants should be named with all
  caps and underlines (same as #define).

  Reply by Dara Navaei on 30 November 2020, 20:38
  > Done

    Reply by Sean Nash on 01 December 2020, 10:51
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 30 November 2020, 13:13 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6358

  This is a one-shot init function, so timing not critical, but I
  would still do this calculation once and assign to local float,
  then assign these fields from the local float.

  Reply by Dara Navaei on 30 November 2020, 20:40
  > Done

    Reply by Sean Nash on 01 December 2020, 10:51
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by qnguyen on 30 November 2020, 14:05 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6398

  We need to set fpga error to 0 to bypass incorrect error count
  from FPGA. Otherwise, there will be temperature sensor fault
  alarm sending to HD once DG is running.

  Reply by Dara Navaei on 30 November 2020, 20:41
  > This was due to an FPGA bug. It has been fixed and I will
  > test it.

    Reply by qnguyen on 01 December 2020, 10:38
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by qnguyen on 07 December 2020, 08:49 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6669

  We need to move this check out of else condition and use the if
  condition. The persistent alarm check function need to be
  switching between true and false cases to work, otherwise we
  need to use windowed time count.

  Reply by Dara Navaei on 07 December 2020, 13:53
  > Done

    Reply by qnguyen on 09 December 2020, 11:40
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by qnguyen on 07 December 2020, 08:51 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6670

  Same comment as above.

  Reply by Dara Navaei on 07 December 2020, 13:53
  > Done

    Reply by qnguyen on 09 December 2020, 11:40
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 30 November 2020, 13:21 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6360

  Looks like a lot of trouble to align these parameters, and
  still not aligned.

  Reply by Dara Navaei on 30 November 2020, 20:41
  > I aligned them.

    Reply by Sean Nash on 01 December 2020, 10:50
    > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: firmware/App/Controllers/Thermistors.c
Revision Comment by Sean Nash on 30 November 2020, 13:54 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6391

  Bit count will never be fractional.  Should declare as integer
  (4096).  If using in a floating point operation, cast it as
  (F32).

  Reply by Dara Navaei on 30 November 2020, 20:44
  > Done. I will cast it.

    Reply by Sean Nash on 01 December 2020, 10:44
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by qnguyen on 30 November 2020, 13:38 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6377

  Persistent alarm has changed to use time period rather than
  count. Change this to time interval.

  Reply by Dara Navaei on 30 November 2020, 20:45
  > Done

    Reply by qnguyen on 07 December 2020, 10:44
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 30 November 2020, 13:50 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6387

  Add "_STATE" to end of each enum name and "state" to end of
  each comment.

  Reply by Dara Navaei on 30 November 2020, 20:47
  > Done

    Reply by Sean Nash on 01 December 2020, 10:44
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 30 November 2020, 13:51 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6389

  Add "_STATE" to end of each enum name and "state" to end of
  each comment.

  Reply by Dara Navaei on 30 November 2020, 20:47
  > Done

    Reply by Sean Nash on 01 December 2020, 10:44
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 30 November 2020, 13:56 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6393

  Our coding standard says constants s/b all caps with underlines
  (same as #defines).

  Reply by Dara Navaei on 30 November 2020, 21:35
  > Done

    Reply by Sean Nash on 01 December 2020, 10:43
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by qnguyen on 30 November 2020, 13:33 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6373

  Align second line.

  Reply by Dara Navaei on 30 November 2020, 20:48
  > Done

    Reply by qnguyen on 01 December 2020, 10:40
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 30 November 2020, 13:58 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6394

  Should explain it's an enum or index for the temperature sensor
  we're getting temperature reading for.

  Reply by Dara Navaei on 30 November 2020, 21:36
  > Done

    Reply by Sean Nash on 01 December 2020, 10:43
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 30 November 2020, 14:01 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6395

  Monitor call(s) should probably be made at top of exec so
  monitoring takes place in all states.

  Reply by Dara Navaei on 30 November 2020, 21:36
  > There are only two states. The other state only waits for a
  > few ms for FPGA to start reading. Also, I didn't want to
  > monitor the thermistors every 50ms.

    Reply by Dara Navaei on 04 December 2020, 09:06
    > Moved the function to execFans function.

      Reply by Sean Nash on 07 December 2020, 10:47
      > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 30 November 2020, 14:02 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6396

  This function should probably be called at top of monitor
  function.

  Reply by Dara Navaei on 30 November 2020, 21:39
  > Done

    Reply by Sean Nash on 01 December 2020, 10:43
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by qnguyen on 07 December 2020, 08:52 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6671

  Need to remove if and else if statement and pass the condition
  to check persistent alarm function.

  Reply by Dara Navaei on 07 December 2020, 13:01
  > I do not understand the comment.

    Reply by qnguyen on 07 December 2020, 19:41
    > The persistent alarm relies on both condition (in range and
    > out of range) to work. So we only need to call it such as
    > "checkPersistentAlarm( persist_alarm_id, temperature <
    > MIN_ALLOWED_TEMPERATURE, temperature,
    > MIN_ALLOWED_TEMPERATURE )" without the if condition.

      Reply by Dara Navaei on 09 December 2020, 08:18
      > Done

        Reply by qnguyen on 09 December 2020, 11:39
        > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 30 November 2020, 14:04 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6397

  This large comment should probably be moved into the function
  brief so SDD can benefit.

  Reply by Dara Navaei on 30 November 2020, 21:41
  > Done

    Reply by Sean Nash on 01 December 2020, 10:42
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by qnguyen on 30 November 2020, 14:13 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6400

  Generally, we have two lines before and after this comment to
  separate test functions and normal code.

  Reply by Dara Navaei on 30 November 2020, 21:41
  > Done

    Reply by qnguyen on 07 December 2020, 10:45
    > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: firmware/source/sys_main.c
Revision Comment by Sean Nash on 30 November 2020, 11:22 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6345

  Move these init functions up in order with other
  controller/monitor init functions.

  Reply by Dara Navaei on 30 November 2020, 21:54
  > Done

    Reply by Sean Nash on 01 December 2020, 10:54
    > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: firmware/App/Controllers/Fans.h
Revision Comment by Sean Nash on 30 November 2020, 13:23 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6361

  Don't need "name" in these enum and type names and maybe add
  "DG" (e.g. DG_Fans, NUM_OF_DG_FANS and DG_FANS_T).  If keeping
  "name", at least fix grammar (e.g. fan_names, NUM_OF_FAN_NAMES
  and FAN_NAMES_T).

  Reply by Dara Navaei on 30 November 2020, 20:25
  > Done

    Reply by Sean Nash on 01 December 2020, 10:50
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 30 November 2020, 13:26 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6362

  Fix comment - s/b PWM duty cycle %.

  Reply by Dara Navaei on 30 November 2020, 20:27
  > I have been changing all the PWM to duty cycle, I just didn't
  > see this one.

    Reply by Sean Nash on 01 December 2020, 10:50
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 30 November 2020, 13:27 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6363

  Change name to getMeasuredFanRPM() to distinguish commanded
  speed from measured speed (even though there is no commanded
  speed in this case).

  Reply by Dara Navaei on 30 November 2020, 20:29
  > I didn't have measured because there is not commanded as you
  > mentioned. But changed the name.

    Reply by Sean Nash on 01 December 2020, 10:50
    > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: firmware/App/Controllers/Thermistors.h
Revision Comment by qnguyen on 30 November 2020, 13:32 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6372

  Remove extra line.

  Reply by Dara Navaei on 30 November 2020, 21:42
  > Done

    Reply by qnguyen on 01 December 2020, 10:40
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 30 November 2020, 13:49 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6385

  Name not needed in enum name.  Consider dg_Thermistors instead.

  Reply by Dara Navaei on 30 November 2020, 21:42
  > Done

    Reply by Sean Nash on 01 December 2020, 10:44
    > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: firmware/App/Drivers/InternalADC.h
Revision Comment by Sean Nash on 30 November 2020, 11:24 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6346

  Are these new channels added to map in InternalADC.c?

  Reply by Dara Navaei on 30 November 2020, 21:45
  > Now added them.

    Reply by Sean Nash on 01 December 2020, 10:54
    > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: firmware/App/Services/FPGA.c
Revision Comment by pmontazemi on 01 December 2020, 08:45 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6479

  There should not be any data received at this time

  Reply by Dara Navaei on 01 December 2020, 09:24
  > You mean change the sentence?

    Reply by Dara Navaei on 04 December 2020, 09:05
    > Done

      Reply by pmontazemi on 07 December 2020, 10:33
      > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: firmware/App/Services/SystemComm.c
Revision Comment by Sean Nash on 30 November 2020, 11:26 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6347

  Why removed?

  Reply by Dara Navaei on 30 November 2020, 21:45
  > It was removed from this branch. It is still available in the
  > RO pump branch.

    Reply by Sean Nash on 01 December 2020, 10:53
    > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: firmware/App/Services/SystemCommMessages.c
Revision Comment by qnguyen on 30 November 2020, 13:29 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6366

  Missing param name.

  Reply by Dara Navaei on 30 November 2020, 21:50
  > I have not seen the parameter name in these functions. I
  > added it.

    Reply by qnguyen on 01 December 2020, 10:41
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by qnguyen on 30 November 2020, 13:29 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6367

  Missing param name.

  Reply by Dara Navaei on 30 November 2020, 21:51
  > Done

    Reply by qnguyen on 01 December 2020, 10:41
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by qnguyen on 07 December 2020, 08:54 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6672

  Mismatch function names.

  Reply by Dara Navaei on 07 December 2020, 12:59
  > Done

    Reply by qnguyen on 09 December 2020, 11:39
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by qnguyen on 07 December 2020, 08:55 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6673

  Mismatch function name.

  Reply by Dara Navaei on 07 December 2020, 12:57
  > Done

    Reply by qnguyen on 09 December 2020, 11:39
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by qnguyen on 07 December 2020, 08:55 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6674

  Mismatch function name.

  Reply by Dara Navaei on 07 December 2020, 12:56
  > Done

    Reply by qnguyen on 09 December 2020, 11:38
    > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: firmware/App/Services/SystemCommMessages.h
Revision Comment by Sean Nash on 30 November 2020, 12:58 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6351

  Alphabetize #includes.

  Reply by Dara Navaei on 30 November 2020, 21:57
  > Done

    Reply by Sean Nash on 01 December 2020, 10:53
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 30 November 2020, 12:59 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6352

  Other broadcast functions appear to be grouped above.

  Reply by Dara Navaei on 30 November 2020, 21:58
  > Ok. I am not sure if git will like this. Done.

    Reply by Sean Nash on 01 December 2020, 10:53
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 30 November 2020, 13:00 
https://devapps.diality.us/cru/DG-DEN-5846-1#c6353

  Keep the word "Test" in these Dialin message handling
  functions.

  Reply by Dara Navaei on 30 November 2020, 22:01
  > Could you please elaborate?

    Reply by Dara Navaei on 04 December 2020, 09:35
    > Done

      Reply by Sean Nash on 07 December 2020, 10:49
      > RESOLVED in CODE WALKTHROUGH.


---

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

Title: DG-DEN-5846_DG Ambient Temperature
Statement of Objectives:


State: Closed

Summary: 

Author: Dara Navaei
Moderator: Dara Navaei
Reviewers: (0 active, 3 completed*)
     qnguyen   (*)
     Sean Nash   (*)
     pmontazemi   (*)