This is a list of all comments for LEAHI-DD-FIRMWARE-LDT-2662-1. Review Summary: No summary ---------------------------------------- File: firmware/App/Monitors/Level.c Revision Comment by Sean Nash on 21 October 2025, 08:38 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2662-1#c24751 HIGH_START is 0, so if != to HIGH_START, count will always be > HIGH_START. So pointless to have that condition in this if statement. Reply by Raghu Kallala on 21 October 2025, 08:41 > [~vmani] Reply by Vinayakam Mani on 21 October 2025, 09:46 > Done. ---------------------------------------- File: firmware/App/Services/Messaging.c Revision Comment by Sean Nash on 06 November 2025, 10:03 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2662-1#c25007 If we're not sending ACK here, let's remove this code and replace with a comment stating we are only ACKing Dialin messages. Reply by Michael Garthwaite on 13 November 2025, 09:56 > fixed. thanks ---------------------------------------- File: firmware/App/Monitors/BloodLeak.c Revision Comment by Sean Nash on 23 October 2025, 15:16 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2662-1#c24789 When are we planning to remove this? Reply by Dara Navaei on 23 October 2025, 16:19 > This is only temporarily showing the FPGA bits of BLD status > for debugging purposes. Reply by Sean Nash on 06 November 2025, 10:04 > Is it still needed? If so, when do we expect it can be > removed? Reply by Dara Navaei on 14 November 2025, 11:23 > No I removed it. ---------------------------------------- File: firmware/App/Monitors/BloodLeak.h Revision Comment by Sean Nash on 23 October 2025, 15:15 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2662-1#c24788 ??? Reply by Dara Navaei on 23 October 2025, 16:19 > This is only temporarily showing the FPGA bits of BLD status > for debugging purposes. Reply by Sean Nash on 06 November 2025, 10:05 > Can we remove it now (before we merge to staging)? Reply by Dara Navaei on 14 November 2025, 11:24 > Done ---------------------------------------- File: firmware/App/Monitors/WaterQualityMonitor.c Revision Comment by Sean Nash on 23 October 2025, 15:14 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2662-1#c24786 Remove extra blank line. Reply by Raghu Kallala on 23 October 2025, 16:15 > Removed. Thanks Revision Comment by Sean Nash on 23 October 2025, 15:14 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2662-1#c24787 Remove extra blank line. Reply by Raghu Kallala on 23 October 2025, 16:16 > Removed. Thanks ---------------------------------------- File: firmware/App/Modes/FPModes/FPOperationModes.c Revision Comment by Sean Nash on 06 November 2025, 09:42 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2662-1#c24996 We shouldn't need to specify the return type in description here. Rephrase to something like "TRUE if FP device is de-featured, FALSE if not". Reply by Michael Garthwaite on 13 November 2025, 09:09 > fixed. thanks Revision Comment by Sean Nash on 06 November 2025, 09:50 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2662-1#c25001 Is it appropriate to call for mode transition here? Better to signal the gen permeate unit (featured or de-featured) and let its state machine handle the transition back to standby mode. Reply by Michael Garthwaite on 12 November 2025, 13:51 > reworked to use the request stop functions in their > respective op modes. Revision Comment by Sean Nash on 06 November 2025, 09:46 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2662-1#c24998 "codes" should not be plural. Reply by Michael Garthwaite on 12 November 2025, 13:50 > fixed. thanks! Revision Comment by Sean Nash on 06 November 2025, 09:47 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2662-1#c24999 Add blank line between declarations and function code. Reply by Michael Garthwaite on 12 November 2025, 13:50 > fixed. thanks! Revision Comment by Sean Nash on 06 November 2025, 09:47 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2662-1#c25000 Should we reject start request if we are not in standby mode? And is it appropriate to call for mode transition here? Better to signal Standby mode and let its state machine handle the transition. Reply by Michael Garthwaite on 12 November 2025, 13:51 > reworked to use the request functions we have in the > respective op modes. They check the standby constraint Revision Comment by Sean Nash on 06 November 2025, 09:46 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2662-1#c24997 Add blank line before return statement. Reply by Michael Garthwaite on 12 November 2025, 13:50 > fixed. thanks! ---------------------------------------- File: firmware/App/Services/TDInterface.c Revision Comment by Sean Nash on 06 November 2025, 10:00 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2662-1#c25004 What are we doing with fpReason? Reply by Michael Garthwaite on 12 November 2025, 15:23 > updated to now change result to FALSE if we get a rr that is > not NONE Reply by Sean Nash on 12 November 2025, 22:28 > Ok, but see my next comment below. Do we need to ACK with > a result at all? Does TD require this? Reply by Michael Garthwaite on 13 November 2025, 09:12 > I think so. the TD needs to know if we are successful in > transitioning to PreGen for either the DD or the FP. > Otherwise, TD is waiting for something else that'll never > happen. > > whether that should be a separate message response or ACK > is up for discussion. Reply by Sean Nash on 14 November 2025, 10:55 > I will resolve this for now, but I don't think ACKs are > necessary and I'm pretty sure TD is not looking for a > response. TD may be waiting for DD to change mode by > looking at DD mode broadcasts. Revision Comment by Sean Nash on 06 November 2025, 09:56 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2662-1#c25003 Is sending of ACK msg needed here? Reply by Michael Garthwaite on 13 November 2025, 09:14 > I think so. the TD needs to know if we are successful in > transitioning to PreGen for either the DD or the FP. > Otherwise, TD is waiting for something else that'll never > happen. > > whether that should be a separate message response or ACK is > up for discussion. Revision Comment by Sean Nash on 06 November 2025, 10:01 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2662-1#c25005 What are we doing with fpReason? Reply by Michael Garthwaite on 12 November 2025, 15:20 > updated to now change result to FALSE if we get a rr that is > not NONE Revision Comment by Sean Nash on 06 November 2025, 10:01 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2662-1#c25006 Is this necessary,? Reply by Michael Garthwaite on 13 November 2025, 09:15 > I think so. the TD needs to know if we are successful in > stopping Gen mode for either the DD or the FP. Otherwise, TD > is waiting for something else that'll never happen. > > whether that should be a separate message response or ACK is > up for discussion. --- ID: LEAHI-DD-FIRMWARE-LDT-2662-1 https://devapps.diality.us/cru/LEAHI-DD-FIRMWARE-LDT-2662-1 Title: LEAHI-DD-FIRMWARE-LDT-2662_Beta 1.0 FW Integration Part 2 - DD Implementation Statement of Objectives: State: Closed Summary: Author: Sean Nash Moderator: Sean Nash Reviewers: (6 active, 2 completed*) Vinayakam Mani (*) Raghu Kallala (*) Vendor - TEL - Arpita Srivastava Vendor - TEL - Jashwant Gantyada Michael Garthwaite Dara Navaei Vendor - TEL - Sivvanarayana Kurapati Daniel Ho