This is a list of all comments for TD-LEAH-32-4. Review Summary: No summary ---------------------------------------- File: firmware/App/Drivers/PressureSensor.c Revision Comment by Vinayakam Mani on 27 August 2024, 16:40 https://devapps.diality.us/cru/TD-LEAH-32-4#c19899 Extra Semicolon. Revision Comment by Vinayakam Mani on 27 August 2024, 17:38 https://devapps.diality.us/cru/TD-LEAH-32-4#c19900 function name needs to be updated. Reply by Sean Nash on 28 August 2024, 08:34 > Fixed. ---------------------------------------- File: firmware/App/Services/FpgaTD.c Revision Comment by Vinayakam Mani on 28 August 2024, 11:42 https://devapps.diality.us/cru/TD-LEAH-32-4#c19963 Needs title and author/date update Reply by Sean Nash on 03 September 2024, 09:25 > Fixed. Reply by Dara Navaei on 04 September 2024, 11:23 > The copyright section will be automatically updated during > the builds. Revision Comment by Vinayakam Mani on 28 August 2024, 11:44 https://devapps.diality.us/cru/TD-LEAH-32-4#c19964 Needs to be updated - TD FPGA ID. Reply by Sean Nash on 28 August 2024, 13:44 > Fixed. Revision Comment by Dara Navaei on 04 September 2024, 11:24 https://devapps.diality.us/cru/TD-LEAH-32-4#c20176 Why the has status register been removed? Reply by Sean Nash on 04 September 2024, 12:30 > Noe didn't include it. Reply by Dara Navaei on 04 September 2024, 12:31 > Do you think we need it? So we should ask Noe to add it? Reply by Sean Nash on 04 September 2024, 21:42 > Some day. I don't even know what it is and if we even > used it in Denali. For now, I want to close this out and > merge to staging. Revision Comment by Vinayakam Mani on 28 August 2024, 12:02 https://devapps.diality.us/cru/TD-LEAH-32-4#c19967 Do we still keep Syring pump for future use as place holder? Reply by Sean Nash on 28 August 2024, 13:28 > I'm just going to follow HDD. If Noe removes these registers > or re-purposes them, I will follow suit. If he leaves them > in, I'll do the same. Revision Comment by Vinayakam Mani on 28 August 2024, 11:57 https://devapps.diality.us/cru/TD-LEAH-32-4#c19966 I guess this is Write register ( not read- back)? you may change name as well to not to confuse with the same name in read page. Reply by Sean Nash on 28 August 2024, 13:38 > Clarified. Revision Comment by Vinayakam Mani on 28 August 2024, 11:50 https://devapps.diality.us/cru/TD-LEAH-32-4#c19965 Reg 07 & 09? may need to update following register series. Reply by Sean Nash on 28 August 2024, 13:40 > Added dummy fields to match addressing specified in HDD. Revision Comment by Vinayakam Mani on 28 August 2024, 12:06 https://devapps.diality.us/cru/TD-LEAH-32-4#c19968 Not matching function names. check the below functions header as well. Reply by Sean Nash on 28 August 2024, 13:26 > Fixed. Revision Comment by Vinayakam Mani on 28 August 2024, 12:10 https://devapps.diality.us/cru/TD-LEAH-32-4#c19969 VBTControl? Reply by Sean Nash on 03 September 2024, 09:34 > This function was for the 4 pinch valves in HD so I don't > think a VBT control is right for this. > I figured we could sort this out when we work on the valves > controller/monitor unit. Revision Comment by Vinayakam Mani on 28 August 2024, 12:11 https://devapps.diality.us/cru/TD-LEAH-32-4#c19970 To be removed? Reply by Sean Nash on 28 August 2024, 13:23 > Removed. ---------------------------------------- File: firmware/App/Drivers/BubbleDetector.c Revision Comment by Vinayakam Mani on 27 August 2024, 15:45 https://devapps.diality.us/cru/TD-LEAH-32-4#c19897 Remove Internal ADC reference. Reply by Sean Nash on 28 August 2024, 08:36 > Fixed. ---------------------------------------- File: firmware/source/sys_main.c Revision Comment by Vinayakam Mani on 28 August 2024, 13:28 https://devapps.diality.us/cru/TD-LEAH-32-4#c19978 Can we shift this into app/driver level code, so that every time we don't touch hal code when we add any new init code? thinking about additional level abstraction (One Init function calls all of these inits and that alone be exposed to hal.) ?? Reply by Sean Nash on 28 August 2024, 14:13 > Yes, I will have controller/monitor call these in their init > functions which will be called here. But until I have those > higher level units, I don't want things to go uninitialized > for now. ---------------------------------------- File: FPGA.c Revision Comment by Vinayakam Mani on 27 August 2024, 11:48 https://devapps.diality.us/cru/TD-LEAH-32-4#c19871 Need to add Params. Reply by Sean Nash on 27 August 2024, 13:29 > Fixed. Revision Comment by Vinayakam Mani on 27 August 2024, 11:54 https://devapps.diality.us/cru/TD-LEAH-32-4#c19872 empty comment section. Reply by Sean Nash on 27 August 2024, 13:24 > Fixed. Revision Comment by Vinayakam Mani on 27 August 2024, 12:01 https://devapps.diality.us/cru/TD-LEAH-32-4#c19873 typo: bulk. Reply by Sean Nash on 27 August 2024, 13:24 > Fixed. Revision Comment by Vinayakam Mani on 27 August 2024, 12:05 https://devapps.diality.us/cru/TD-LEAH-32-4#c19874 Do we need to add alarm for DD and RO as well? there are few functions down needs the same resolution. Reply by Sean Nash on 27 August 2024, 13:26 > Fixed. Revision Comment by Vinayakam Mani on 30 August 2024, 15:15 https://devapps.diality.us/cru/TD-LEAH-32-4#c20110 Need to add DD and RO. Reply by Sean Nash on 03 September 2024, 09:17 > Done. Revision Comment by Vinayakam Mani on 30 August 2024, 15:15 https://devapps.diality.us/cru/TD-LEAH-32-4#c20111 Need to add DD and RO. Reply by Sean Nash on 03 September 2024, 09:17 > Done. Revision Comment by Vinayakam Mani on 30 August 2024, 15:15 https://devapps.diality.us/cru/TD-LEAH-32-4#c20112 Need to add DD and RO. Reply by Sean Nash on 03 September 2024, 09:17 > Done. ---------------------------------------- File: PressureCommon.c Revision Comment by Vinayakam Mani on 27 August 2024, 13:46 https://devapps.diality.us/cru/TD-LEAH-32-4#c19888 are we handling negative counts here? Per datasheet, If the count value is more than 8388608, Actual count value is "count -16777216". Reply by Sean Nash on 27 August 2024, 14:29 > I believe FPGA will have already sign extended the reading to > 32-bits for us, so we should not be required to do those > steps. I will confirm with Noe. ---------------------------------------- File: firmware/App/Controllers/AlarmAudio.c Revision Comment by Vinayakam Mani on 27 August 2024, 15:23 https://devapps.diality.us/cru/TD-LEAH-32-4#c19896 Alarm Audio. Reply by Sean Nash on 28 August 2024, 08:37 > Fixed. ---------------------------------------- File: firmware/App/Controllers/AlarmAudio.h Revision Comment by Vinayakam Mani on 27 August 2024, 15:51 https://devapps.diality.us/cru/TD-LEAH-32-4#c19898 AlarmLamp - rename as AlarmAudio Reply by Sean Nash on 28 August 2024, 08:36 > Fixed. ---------------------------------------- File: firmware/App/Drivers/PAL.c Revision Comment by Dara Navaei on 04 September 2024, 11:15 https://devapps.diality.us/cru/TD-LEAH-32-4#c20173 Add more details. Reply by Sean Nash on 04 September 2024, 12:25 > Details are in the header file where the group is defined. > This is just saying that all of the code below goes into this > group. ---------------------------------------- File: firmware/App/Modes/ModeFault.c Revision Comment by Dara Navaei on 04 September 2024, 11:12 https://devapps.diality.us/cru/TD-LEAH-32-4#c20172 Remove this code. There is no RTC anymore. Reply by Sean Nash on 04 September 2024, 12:28 > Removed. ---------------------------------------- File: firmware/App/Services/AlarmMgmtTD.c Revision Comment by Dara Navaei on 04 September 2024, 11:18 https://devapps.diality.us/cru/TD-LEAH-32-4#c20174 Can some of these functions be added to the leahi-fwcommon alarm management? Like active alarm or clear alarm. I understand that it might be too difficult to tailor these functions for all of the stacks. Reply by Sean Nash on 04 September 2024, 12:29 > I kept these out of common because it's doing some specific > things like broadcasting alarm trigger message. Revision Comment by Vinayakam Mani on 28 August 2024, 11:19 https://devapps.diality.us/cru/TD-LEAH-32-4#c19958 may need to add alarmstatus.alarmsSilenceStart and ExpiresIn attributes part of outputs. Reply by Sean Nash on 28 August 2024, 14:12 > Fixed. ---------------------------------------- File: firmware/App/Services/CpldInterface.c Revision Comment by Vinayakam Mani on 28 August 2024, 11:30 https://devapps.diality.us/cru/TD-LEAH-32-4#c19959 file name CpldInterface.c Reply by Sean Nash on 03 September 2024, 09:30 > Done. Revision Comment by Vinayakam Mani on 28 August 2024, 11:33 https://devapps.diality.us/cru/TD-LEAH-32-4#c19960 looks this feature is not supported in Leahi ( per last discussion On power management review)?? Revision Comment by Vinayakam Mani on 28 August 2024, 11:35 https://devapps.diality.us/cru/TD-LEAH-32-4#c19961 Do we still support this in Leahi? I heard in the last power management meeting that it is likely removed. Reply by Sean Nash on 28 August 2024, 14:09 > I think that's right. Waiting for schematic to see how it > will work with Leahi. ---------------------------------------- File: firmware/App/Services/DDInterface.c Revision Comment by Vinayakam Mani on 28 August 2024, 11:38 https://devapps.diality.us/cru/TD-LEAH-32-4#c19962 DG -> DD. may clean up in entire file. Reply by Sean Nash on 28 August 2024, 14:11 > Fixed. ---------------------------------------- File: firmware/App/Services/Messaging.c Revision Comment by Vinayakam Mani on 28 August 2024, 13:10 https://devapps.diality.us/cru/TD-LEAH-32-4#c19971 \b alarm (to bold Reply by Sean Nash on 28 August 2024, 13:22 > Fixed. ---------------------------------------- File: firmware/App/Services/SystemCommTD.c Revision Comment by Vinayakam Mani on 28 August 2024, 13:19 https://devapps.diality.us/cru/TD-LEAH-32-4#c19972 DG->DD Reply by Sean Nash on 28 August 2024, 13:21 > Fixed. ---------------------------------------- File: AlarmDefs.h Revision Comment by Vinayakam Mani on 27 August 2024, 09:49 https://devapps.diality.us/cru/TD-LEAH-32-4#c19847 I guess we need to rename as DD fault. Also, can we introduce RO fault parameter? This would help us to differentiate RO alarms when plugged into DD. Reply by Sean Nash on 03 September 2024, 09:27 > Renamed to DD fault. > I think an RO fault can share the DD fault property since I > imagine an RO fault would cause the DD to go to fault mode. Revision Comment by Vinayakam Mani on 27 August 2024, 10:26 https://devapps.diality.us/cru/TD-LEAH-32-4#c19858 Looks description needs update for both Arterial and Venous Sensor Time out fault. Reply by Sean Nash on 27 August 2024, 13:20 > Fixed. ---------------------------------------- File: AlarmMgmt.c Revision Comment by Vinayakam Mani on 27 August 2024, 11:03 https://devapps.diality.us/cru/TD-LEAH-32-4#c19867 Remove extra semicolon Reply by Sean Nash on 27 August 2024, 13:30 > Fixed. Revision Comment by Vinayakam Mani on 27 August 2024, 11:12 https://devapps.diality.us/cru/TD-LEAH-32-4#c19868 typo: alarm rank table. Reply by Sean Nash on 27 August 2024, 13:20 > Fixed. Revision Comment by Vinayakam Mani on 27 August 2024, 11:19 https://devapps.diality.us/cru/TD-LEAH-32-4#c19869 add in Outputs: alarmIsDetected[] Reply by Sean Nash on 27 August 2024, 13:21 > Fixed. Revision Comment by Vinayakam Mani on 27 August 2024, 11:25 https://devapps.diality.us/cru/TD-LEAH-32-4#c19870 Generic name (XX) instead of TD. Reply by Sean Nash on 27 August 2024, 13:22 > Fixed. ---------------------------------------- File: PersistentAlarm.c Revision Comment by Dara Navaei on 04 September 2024, 10:49 https://devapps.diality.us/cru/TD-LEAH-32-4#c20168 This line needs a doxygen comment. Reply by Sean Nash on 04 September 2024, 12:15 > Why? I don't recall commenting on private function > prototypes. We comment them in function headers. ---------------------------------------- File: SystemComm.c Revision Comment by Vinayakam Mani on 27 August 2024, 14:03 https://devapps.diality.us/cru/TD-LEAH-32-4#c19889 DG to DD. Reply by Sean Nash on 27 August 2024, 14:30 > Fixed. Revision Comment by Vinayakam Mani on 27 August 2024, 14:09 https://devapps.diality.us/cru/TD-LEAH-32-4#c19890 Alarm for DD and RO. Reply by Sean Nash on 27 August 2024, 14:32 > Fixed. Revision Comment by Vinayakam Mani on 27 August 2024, 14:38 https://devapps.diality.us/cru/TD-LEAH-32-4#c19894 Alarm for DD and RO. Reply by Sean Nash on 04 September 2024, 12:17 > Fixed. Revision Comment by Dara Navaei on 04 September 2024, 10:54 https://devapps.diality.us/cru/TD-LEAH-32-4#c20169 Per the design we agreed on TD not checking with RO directly, right? Reply by Sean Nash on 04 September 2024, 12:17 > Correct. ---------------------------------------- File: TestSupport.c Revision Comment by Vinayakam Mani on 27 August 2024, 14:45 https://devapps.diality.us/cru/TD-LEAH-32-4#c19895 No alarms in the function. check the below one as well. Reply by Sean Nash on 28 August 2024, 08:38 > Fixed. ---------------------------------------- File: Utilities.c Revision Comment by Dara Navaei on 04 September 2024, 10:58 https://devapps.diality.us/cru/TD-LEAH-32-4#c20170 We should have #define for this number. Reply by Sean Nash on 04 September 2024, 12:18 > Who wrote this function? Do you know what to call it? Reply by Dara Navaei on 04 September 2024, 12:29 > This function is used to calculate the baro sensor's CRC. Reply by Sean Nash on 04 September 2024, 21:45 > But what is 0x3000? Reply by Dara Navaei on 04 September 2024, 22:06 > I can update the numbers. Revision Comment by Dara Navaei on 04 September 2024, 10:58 https://devapps.diality.us/cru/TD-LEAH-32-4#c20171 We should have #define for this number. Reply by Sean Nash on 04 September 2024, 12:22 > This one masks off lowest nibble. I will make a #define for > it. --- ID: TD-LEAH-32-4 https://devapps.diality.us/cru/TD-LEAH-32-4 Title: TD-LEAH-32_FW TD Fpga Interface Statement of Objectives: State: Closed Summary: Author: Sean Nash Moderator: Sean Nash Reviewers: (2 active, 2 completed*) Vinayakam Mani (*) Dara Navaei (*) jpaguio Michael Garthwaite