This is a list of all comments for DG-DEN-1125-1. Review Summary: No summary General Comment by Dara Navaei on 13 December 2019, 08:40 https://devapps.diality.us/cru/DG-DEN-1125-1#c585 The .gitignore for the DG firmware must be like the .gitignore for the HD firmware. Reply by Dara Navaei on 07 January 2020, 17:15 > RESOLVED IN CODE WALKTHROUGH General Comment by lbaloa on 16 December 2019, 10:24 https://devapps.diality.us/cru/DG-DEN-1125-1#c608 .gitignore is due to a /document file that I have in local folder. We need flexibility for developers. Reply by Dara Navaei on 07 January 2020, 17:16 > RESOLVED IN CODE WALKTHROUGH General Comment by pmontazemi on 26 December 2019, 11:07 https://devapps.diality.us/cru/DG-DEN-1125-1#c702 Add *.pyc files to .gitignore. Reply by pmontazemi on 07 January 2020, 17:15 > RESOLVED IN CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/SystemCommMessages.c Revision Comment by Sean Nash on 13 December 2019, 08:43 https://devapps.diality.us/cru/DG-DEN-1125-1#c587 Why does DG have an off button message? Reply by lbaloa on 16 December 2019, 10:25 > It is just for now. Cleaning needs to happen. Reply by pmontazemi on 17 December 2019, 13:35 > Remove OFF Button message. Reply by lbaloa on 20 December 2019, 11:35 > Done! Reply by Sean Nash on 07 January 2020, 17:20 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 13 December 2019, 08:44 https://devapps.diality.us/cru/DG-DEN-1125-1#c588 All references to "cargo" should be changed to "payload". Reply by lbaloa on 16 December 2019, 10:23 > Done Reply by Sean Nash on 20 December 2019, 10:07 > RESOLVED IN CODE WALKTHROUGH Revision Comment by Sean Nash on 10 December 2019, 12:51 https://devapps.diality.us/cru/DG-DEN-1125-1#c523 Body brackets should be on separate line per our coding standard. Reply by lbaloa on 11 December 2019, 10:14 > Done Reply by Sean Nash on 11 December 2019, 14:28 > RESOLVED IN CODE WALKTHROUGH Reply by pmontazemi on 11 December 2019, 14:30 > Still see curly brackets in line with code! Reply by lbaloa on 12 December 2019, 08:47 > Peman, please read e-mail. The changes cannot be seen > here because issues we are having. You need to see > changes in bitbucket. Reply by lbaloa on 16 December 2019, 10:32 > Done Reply by Sean Nash on 20 December 2019, 10:18 > RESOLVED IN CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Modes/ModeFill.c Revision Comment by Sean Nash on 10 December 2019, 12:46 https://devapps.diality.us/cru/DG-DEN-1125-1#c522 Copy & paste from one of my mode modules? Change to when you created this file. Reply by lbaloa on 11 December 2019, 10:20 > Done Reply by Sean Nash on 11 December 2019, 14:29 > RESOLVED IN CODE WALKTHROUGH Revision Comment by Sean Nash on 13 December 2019, 08:36 https://devapps.diality.us/cru/DG-DEN-1125-1#c582 "{" should be on separate line (throughout code). Reply by lbaloa on 20 December 2019, 14:24 > Done! Reply by lbaloa on 06 January 2020, 08:51 > Done Reply by Sean Nash on 07 January 2020, 17:22 > RESOLVED in CODE WALKTHROUGH. Revision Comment by pmontazemi on 30 December 2019, 08:37 https://devapps.diality.us/cru/DG-DEN-1125-1#c730 Remove extra line. Reply by lbaloa on 06 January 2020, 08:50 > Done Reply by pmontazemi on 07 January 2020, 17:17 > RESOLVED IN CODE WALKTHROUGH. Revision Comment by pmontazemi on 30 December 2019, 08:37 https://devapps.diality.us/cru/DG-DEN-1125-1#c731 Remove extra line. Reply by lbaloa on 06 January 2020, 08:47 > Done Reply by pmontazemi on 07 January 2020, 17:17 > RESOLVED IN CODE WALKTHROUGH. Revision Comment by pmontazemi on 30 December 2019, 08:37 https://devapps.diality.us/cru/DG-DEN-1125-1#c732 Remove extra line. Reply by lbaloa on 06 January 2020, 08:47 > Done Reply by pmontazemi on 07 January 2020, 17:17 > RESOLVED IN CODE WALKTHROUGH. Revision Comment by pmontazemi on 30 December 2019, 08:37 https://devapps.diality.us/cru/DG-DEN-1125-1#c733 Remove extra line. Reply by lbaloa on 06 January 2020, 08:47 > Done Reply by pmontazemi on 07 January 2020, 17:17 > RESOLVED IN CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/ModeStandby.c Revision Comment by pmontazemi on 11 December 2019, 14:29 https://devapps.diality.us/cru/DG-DEN-1125-1#c546 This method is left empty. Purpose? Reply by lbaloa on 16 December 2019, 10:57 > Yes, at the moment, standby doesn't do anything. Reply by pmontazemi on 20 December 2019, 10:23 > RESOLVED in CODE WALKTHROUGH. Revision Comment by pmontazemi on 20 December 2019, 10:23 https://devapps.diality.us/cru/DG-DEN-1125-1#c694 Stop Button removed because none on DG. Reply by lbaloa on 20 December 2019, 11:13 > Done! Reply by pmontazemi on 07 January 2020, 17:18 > RESOLVED IN CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/OperationModes.c Revision Comment by Sean Nash on 10 December 2019, 12:56 https://devapps.diality.us/cru/DG-DEN-1125-1#c525 Leo should take ownership of DG modules that are not going to be common with HD. Change author and date. Reply by lbaloa on 11 December 2019, 09:54 > Done Reply by Sean Nash on 11 December 2019, 14:22 > RESOLVED IN CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Modes/OperationModes.h Revision Comment by Sean Nash on 13 December 2019, 08:38 https://devapps.diality.us/cru/DG-DEN-1125-1#c583 When will this reflect the DG modes from DG SRS? Reply by lbaloa on 16 December 2019, 10:29 > In the next DG story. I would line up DG with its > implementation better. Reply by Sean Nash on 07 January 2020, 17:21 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/SystemComm.c Revision Comment by Sean Nash on 13 December 2019, 08:41 https://devapps.diality.us/cru/DG-DEN-1125-1#c586 I just had to go through all my HD code to address spacing. Should have spaces in brackets ( [ or ( or { ) and between items in lists. Reply by lbaloa on 16 December 2019, 10:27 > Because this will be a common file. I don't think it will be > worth to go through that exercise. If this was unique with > DG, I would agree with the comment. Reply by pmontazemi on 17 December 2019, 13:34 > Leo, please make spacing consistent throughout the entire > DG code base, this is a must because of our updated C > coding standard. Thanks! Reply by lbaloa on 20 December 2019, 11:43 > Done! Reply by Sean Nash on 07 January 2020, 17:21 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Dara Navaei on 10 December 2019, 13:16 https://devapps.diality.us/cru/DG-DEN-1125-1#c531 Why do we have extra lines here? Reply by lbaloa on 11 December 2019, 09:52 > Done Reply by Dara Navaei on 26 December 2019, 11:26 > RESOLVED in CODEWALKTHROUGH ---------------------------------------- File: firmware/App/Services/SystemCommMessages.h Revision Comment by pmontazemi on 11 December 2019, 14:31 https://devapps.diality.us/cru/DG-DEN-1125-1#c550 Remove Off Button from DG. Reply by lbaloa on 12 December 2019, 08:36 > This is an enum, not sure what you mean to initialize. > MSG_ID_OFF_BUTTON_PRESS, is set to 1 Reply by pmontazemi on 17 December 2019, 13:36 > If it is set to 1 as default, shouldn't it be = 1? Reply by lbaloa on 20 December 2019, 14:30 > Done Reply by pmontazemi on 07 January 2020, 17:18 > RESOLVED IN CODE WALKTHROUGH. Revision Comment by pmontazemi on 11 December 2019, 14:31 https://devapps.diality.us/cru/DG-DEN-1125-1#c549 Space between = and 1000 Reply by pmontazemi on 20 December 2019, 10:20 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Sean Nash on 10 December 2019, 12:54 https://devapps.diality.us/cru/DG-DEN-1125-1#c524 DG dialin message IDs should start at some other offset. HD is using 0x8000. DG should maybe use 0xA000. Reply by lbaloa on 11 December 2019, 10:11 > Done. Other IDs will be eliminated as we move along. Reply by Sean Nash on 11 December 2019, 14:24 > I see a change to 0xA000, but then another 0xA100. I > assume this is normal messages starting at 0xA000 and > dialin at 0xA100? I would recommend starting DG normal > messages at something lower like 0x2000 and then DG dialin > messages at 0xA000. Reply by lbaloa on 16 December 2019, 10:55 > Done. Moved main messages to 0x2000 and dialin messages > to 0xA000 Reply by Sean Nash on 20 December 2019, 10:19 > RESOLVED IN CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Services/CommBuffers.h Revision Comment by Sean Nash on 13 December 2019, 08:39 https://devapps.diality.us/cru/DG-DEN-1125-1#c584 I think you should add the HD Alarm channel (as an "IN") so you get the system alarm status. Reply by lbaloa on 16 December 2019, 10:28 > I need to do a major clean up, at this time, this change > requires major clean up. Reply by Sean Nash on 20 December 2019, 10:09 > Will add later if needed. RESOLVED IN CODE WALKTHROUGH ---------------------------------------- File: .gitignore Revision Comment by Dara Navaei on 13 December 2019, 08:45 https://devapps.diality.us/cru/DG-DEN-1125-1#c589 Why do we have this file here? Each VectorCAST and firmware must have their own .gitignore in their folder. Reply by lbaloa on 13 December 2019, 14:20 > I do have a document folder where I check for doxygen output > locally Reply by Dara Navaei on 20 December 2019, 10:05 > RESOLVED IN CODE WALKTHROUGH --- ID: DG-DEN-1125-1 https://devapps.diality.us/cru/DG-DEN-1125-1 Title: DG-DEN-1125_DG Standby / Fill State Machine Statement of Objectives: State: Closed Summary: Author: lbaloa Moderator: lbaloa Reviewers: (0 active, 3 completed*) Sean Nash (*) Dara Navaei (*) pmontazemi (*)