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

Review Summary: No summary


----------------------------------------
File: firmware/App/Services/FPGA.c
Revision Comment by Sean Nash on 28 May 2021, 15:19 
https://devapps.diality.us/cru/DG-DEN-8103-1#c10135

  Why did this change?

  Reply by qnguyen on 28 May 2021, 16:31
  > Noe said they updated the ID and this is the new ID for DG
  > FPGA.
  > The FPGA test will fail without this change.

    Reply by Sean Nash on 03 June 2021, 10:35
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 28 May 2021, 15:20 
https://devapps.diality.us/cru/DG-DEN-8103-1#c10136

  Should mention somewhere that timer is counting ms.

  Reply by qnguyen on 28 May 2021, 16:39
  > Done.

    Reply by Sean Nash on 03 June 2021, 10:35
    > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: firmware/App/Services/AlarmMgmt.h
Revision Comment by Sean Nash on 19 May 2021, 17:15 
https://devapps.diality.us/cru/DG-DEN-8103-1#c9981

  Should we move this enum to separate .h file?

  Reply by qnguyen on 27 May 2021, 10:34
  > Done.

    Reply by Sean Nash on 03 June 2021, 10:36
    > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: firmware/App/Controllers/Heaters.c
Revision Comment by pmontazemi on 19 May 2021, 15:35 
https://devapps.diality.us/cru/DG-DEN-8103-1#c9965

  Place TRUE as first argument.

  Reply by qnguyen on 20 May 2021, 11:24
  > The only preference is to put TRUE first if it is == such as
  > below.

    Reply by pmontazemi on 27 May 2021, 10:30
    > RESOLVED in CODE WALKTHROUGH.

----------------------------------------
File: firmware/App/Services/Integrity.c
Revision Comment by Sean Nash on 28 May 2021, 15:14 
https://devapps.diality.us/cru/DG-DEN-8103-1#c10133

  Is there a reason why this module isn't in fwcommon?  And if it
  can't be common for some reason, should this module (or group
  name) be named DGIntegrity?

  Reply by qnguyen on 29 May 2021, 01:16
  > Good point. The module has been moved to fwcommon.

    Reply by Sean Nash on 03 June 2021, 10:36
    > RESOLVED in CODE WALKTHROUGH.

Revision Comment by Sean Nash on 28 May 2021, 15:18 
https://devapps.diality.us/cru/DG-DEN-8103-1#c10134

  Do we need to support more than one 32-bit CRC algorithm in
  firmware?  If not, do we need this wrapper function?

  Reply by qnguyen on 29 May 2021, 01:16
  > Removed the wrapper function. For now, only support one type
  > of 32-bit CRC algorithm.

    Reply by Sean Nash on 03 June 2021, 10:36
    > RESOLVED in CODE WALKTHROUGH.


---

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

Title: DG-DEN-8103_DG HD Dev Alarm Design
Statement of Objectives:


State: Closed

Summary: 

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