This is a list of all comments for HD-DEN-16565-1. Review Summary: No summary ---------------------------------------- File: firmware/.cproject Revision Comment by Sean Nash on 22 December 2023, 09:08 https://devapps.diality.us/cru/HD-DEN-16565-1#c19325 This is adding conversion to binary step post-build, correct? Are we losing the alarm check here? Revision Comment by Sean Nash on 22 December 2023, 09:09 https://devapps.diality.us/cru/HD-DEN-16565-1#c19326 What is this for? ---------------------------------------- File: firmware/source/sys_link.cmd Revision Comment by Sean Nash on 22 December 2023, 09:02 https://devapps.diality.us/cru/HD-DEN-16565-1#c19323 So app start address is offset by 64K. Assuming that is plenty of room for bootloader but we should check. Also, have we checked that the first 64K of Flash memory is discreet sector-wise? We do not want bootloader and app sharing a sector. Revision Comment by Vinayakam Mani on 24 May 2024, 11:52 https://devapps.diality.us/cru/HD-DEN-16565-1#c19779 Bootloader seems to be executed in supervisory mode. if so, the current allocated stack for supervisory mode is 1K (if we merge into existing HD/DG firmware). we may need to watch for supervisory mode stack size, in case boot loader consumes more that 1K? Reply by Sean Nash on 06 June 2024, 10:52 > Why is bootloader executing in a different mode than app? Is > that necessary? How will mode switch back to user when > jumping to app? ---------------------------------------- File: firmware/App/Modes/Update.c Revision Comment by Sean Nash on 22 December 2023, 08:45 https://devapps.diality.us/cru/HD-DEN-16565-1#c19307 This doesn't look like a mode - it looks like a small collection of services - suggest moving to Services folder. Revision Comment by Sean Nash on 22 December 2023, 08:48 https://devapps.diality.us/cru/HD-DEN-16565-1#c19309 Need doxygen group stuff here. Revision Comment by Vinayakam Mani on 24 May 2024, 11:43 https://devapps.diality.us/cru/HD-DEN-16565-1#c19778 the second value is 0x00 or 0xFFFFFFFF? The bootloader checks the value is not equal to 0xFFFFFFFF, if so, the valid application is not found, and firmware needs to be updated. Revision Comment by Sean Nash on 22 December 2023, 08:47 https://devapps.diality.us/cru/HD-DEN-16565-1#c19308 Why do we need this function? Why not just use the reset function below to start the bootloader? Revision Comment by Sean Nash on 22 December 2023, 09:29 https://devapps.diality.us/cru/HD-DEN-16565-1#c19327 Do we really want to use a whole flash sector for a flag? I thought bootloader was going to talk to UI to determine whether an update was in progress? Revision Comment by Vinayakam Mani on 24 May 2024, 11:19 https://devapps.diality.us/cru/HD-DEN-16565-1#c19777 Looks to be memory overflow to be occurred. src-> pointing the buffer of 4 bytes length. bytes-> mentioning 16 bytes length. g_pulUpdateSuccess[] should be allocated more than the current size. Also, Do we need to typecast "(uint32_t) bytes", since the api mentioning (uint8_t)? Fapi_StatusType Fapi_issueProgrammingCommand( uint32_t *pu32StartAddress, uint8_t *pu8DataBuffer, uint8_t u8DataBufferSizeInBytes, uint8_t *pu8EccBuffer, uint8_t u8EccBufferSizeInBytes, Fapi_FlashProgrammingCommandType oMode) ---------------------------------------- File: firmware/App/Modes/Update.h Revision Comment by Sean Nash on 22 December 2023, 08:49 https://devapps.diality.us/cru/HD-DEN-16565-1#c19310 Need doxygen group stuff here. ---------------------------------------- File: firmware/App/Services/SystemComm.c Revision Comment by Sean Nash on 22 December 2023, 08:51 https://devapps.diality.us/cru/HD-DEN-16565-1#c19311 These changes look wrong. This is UI comm check. Revision Comment by Sean Nash on 22 December 2023, 09:00 https://devapps.diality.us/cru/HD-DEN-16565-1#c19322 Why aren't each of these messages calling their own handler function? They appear to exist in SystemCommMessages.c so I believe that was the intent. ---------------------------------------- File: firmware/App/Services/SystemCommMessages.c Revision Comment by Sean Nash on 22 December 2023, 08:59 https://devapps.diality.us/cru/HD-DEN-16565-1#c19321 Should we use new reboot function in Update.c? Revision Comment by Sean Nash on 22 December 2023, 08:54 https://devapps.diality.us/cru/HD-DEN-16565-1#c19313 Where is this called? Revision Comment by Sean Nash on 22 December 2023, 08:58 https://devapps.diality.us/cru/HD-DEN-16565-1#c19320 Spaces in (). Revision Comment by Sean Nash on 22 December 2023, 08:55 https://devapps.diality.us/cru/HD-DEN-16565-1#c19314 This message will not get sent immediately - it is only queued. Reset will occur before transmit. Revision Comment by Sean Nash on 22 December 2023, 08:55 https://devapps.diality.us/cru/HD-DEN-16565-1#c19315 Spaces in (). Be explicit on BOOL conditions. Revision Comment by Sean Nash on 22 December 2023, 08:56 https://devapps.diality.us/cru/HD-DEN-16565-1#c19316 Where is this being called? Revision Comment by Sean Nash on 22 December 2023, 08:56 https://devapps.diality.us/cru/HD-DEN-16565-1#c19317 Spaces inside (). Use ternary to set to TRUE or FALSE explicitly. Revision Comment by Sean Nash on 22 December 2023, 08:57 https://devapps.diality.us/cru/HD-DEN-16565-1#c19318 Spaces inside (). Be explicit with BOOL conditions. Revision Comment by Sean Nash on 22 December 2023, 08:57 https://devapps.diality.us/cru/HD-DEN-16565-1#c19319 Why not call the reboot function in Update.c? ---------------------------------------- File: firmware/App/Services/SystemCommMessages.h Revision Comment by Sean Nash on 22 December 2023, 08:53 https://devapps.diality.us/cru/HD-DEN-16565-1#c19312 I don't see SystemComm calling this or reboot function. Why are they here? Revision Comment by Darren Cox on 25 January 2024, 14:32 https://devapps.diality.us/cru/HD-DEN-16565-1#c19361 These are not in HD where all go to one function. Should this be common code between DG and HD? ---------------------------------------- File: firmware/source/sys_startup.c Revision Comment by Sean Nash on 22 December 2023, 09:06 https://devapps.diality.us/cru/HD-DEN-16565-1#c19324 Don't make changes outside of user code sections. ---------------------------------------- File: firmware/.settings/com.ti.ccstudio.project.core.prefs Revision Comment by Sean Nash on 22 December 2023, 08:44 https://devapps.diality.us/cru/HD-DEN-16565-1#c19306 Are these changes necessary? What do they do exactly? --- ID: HD-DEN-16565-1 https://devapps.diality.us/cru/HD-DEN-16565-1 Title: HD-DEN-16565_SW Updater HD DG App Side Intake From Sunrise Statement of Objectives: State: Closed Summary: Author: Dara Navaei Moderator: Dara Navaei Reviewers: (6 active, 0 completed*) Sean Nash jpaguio Vinayakam Mani Michael Garthwaite Darren Cox Dana Rich