This is a list of all comments for HD-DEN-9906-1. Review Summary: No summary ---------------------------------------- File: firmware/App/Modes/ModeTreatment.c Revision Comment by hnguyen on 26 October 2021, 13:39 https://devapps.diality.us/cru/HD-DEN-9906-1#c10927 per MISRA, need the else statement at the end of if { }- else if { } - else if { }- else { } Reply by Sean Nash on 15 November 2021, 14:59 > Our coding standard does not require MISRA compliance and I > don't think our coding standard includes this if else rule. > If you think it should be added to our standard, let's > discuss. Reply by hnguyen on 15 November 2021, 17:03 > RESOLVED in CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Services/SystemComm.c Revision Comment by Behrouz NematiPour on 26 October 2021, 21:41 https://devapps.diality.us/cru/HD-DEN-9906-1#c10944 Could you please explain why the ack timeout increased, is it for POST ack issues or there were other things too? Reply by Sean Nash on 27 October 2021, 08:37 > I was thinking 150 was perhaps too fast for UI. But you're > right - that only seems to be true at startup. I'll put back > to 150. Reply by Behrouz NematiPour on 27 October 2021, 10:40 > RESOLVED IN CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/BloodFlow.c Revision Comment by hnguyen on 15 November 2021, 12:38 https://devapps.diality.us/cru/HD-DEN-9906-1#c11402 Add variable in line 386 to @detail Inputs & Outputs Reply by Sean Nash on 15 November 2021, 16:49 > Done. Reply by hnguyen on 15 November 2021, 17:05 > RESOLVED in CODE WALKTHROUGH Revision Comment by hnguyen on 15 November 2021, 12:45 https://devapps.diality.us/cru/HD-DEN-9906-1#c11403 Inputs: should be None Outputs: Alarm may triggered Reply by Sean Nash on 15 November 2021, 16:48 > Done. Reply by hnguyen on 15 November 2021, 17:04 > RESOLVED in CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Controllers/DialInFlow.c Revision Comment by hnguyen on 15 November 2021, 13:04 https://devapps.diality.us/cru/HD-DEN-9906-1#c11406 Some of local variables are not used in this function, consider removing them. Reply by Sean Nash on 15 November 2021, 16:44 > Removed. Reply by hnguyen on 15 November 2021, 17:04 > RESOLVED in CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Controllers/Fans.c Revision Comment by hnguyen on 26 October 2021, 13:15 https://devapps.diality.us/cru/HD-DEN-9906-1#c10918 This file is missing a file header Reply by Sean Nash on 01 November 2021, 10:19 > Added file header. Reply by hnguyen on 10 November 2021, 11:06 > RESOLVED in CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Controllers/PresOccl.c Revision Comment by hnguyen on 15 November 2021, 13:45 https://devapps.diality.us/cru/HD-DEN-9906-1#c11408 Outputs: Alarm may be activated Reply by Sean Nash on 15 November 2021, 14:58 > Added comment. Reply by hnguyen on 15 November 2021, 17:02 > RESOLVED in CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Controllers/Switches.c Revision Comment by hnguyen on 26 October 2021, 13:16 https://devapps.diality.us/cru/HD-DEN-9906-1#c10919 Add file header Reply by Sean Nash on 27 October 2021, 08:43 > Fixed. Reply by hnguyen on 27 October 2021, 10:47 > RESOLVED IN CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Controllers/SyringePump.c Revision Comment by Behrouz NematiPour on 26 October 2021, 22:23 https://devapps.diality.us/cru/HD-DEN-9906-1#c10950 Shouldn't be? {code:C} ( FALSE == isBloodPumpRunning() ) {code} Reply by Sean Nash on 27 October 2021, 08:28 > FALSE is simply zero and TRUE is one. In case a boolean is > somehow corrupted to something other than 1 or 0, I like to > err on the side of safety. > So the way I have it now, if isBloodPumpRunning() gets > corrupted to > 1, it will consider that the pump is not > running which is the safer thing to do. Reply by Behrouz NematiPour on 27 October 2021, 10:36 > RESOLVED IN CODE WALKTHROUGH. Revision Comment by Dara Navaei on 15 November 2021, 12:58 https://devapps.diality.us/cru/HD-DEN-9906-1#c11404 This should an F64. Reply by Sean Nash on 15 November 2021, 16:46 > Done. Reply by Dara Navaei on 15 November 2021, 16:58 > RESOLVED in CODE WALKTHROUGH. Revision Comment by Dara Navaei on 15 November 2021, 13:01 https://devapps.diality.us/cru/HD-DEN-9906-1#c11405 This should be an F64. Reply by Sean Nash on 15 November 2021, 16:46 > Done. Reply by Dara Navaei on 15 November 2021, 16:58 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Controllers/Temperatures.c Revision Comment by hnguyen on 26 October 2021, 13:19 https://devapps.diality.us/cru/HD-DEN-9906-1#c10923 Add file header Reply by Sean Nash on 27 October 2021, 08:49 > Fixed. Reply by hnguyen on 27 October 2021, 10:48 > RESOLVED IN CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Controllers/Voltages.c Revision Comment by Dara Navaei on 15 November 2021, 13:10 https://devapps.diality.us/cru/HD-DEN-9906-1#c11407 Adjust the space before the equal signs. Reply by Sean Nash on 15 November 2021, 15:01 > I adjusted spaces "before" the equal sign. Reply by Dara Navaei on 15 November 2021, 16:58 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/Dialysis.c Revision Comment by Behrouz NematiPour on 05 November 2021, 19:06 https://devapps.diality.us/cru/HD-DEN-9906-1#c11135 I couldn't find TBD anywhere in the code!!! What is TBD? Reply by Sean Nash on 12 November 2021, 11:35 > Finished implementing the TBD here. Reply by Dara Navaei on 15 November 2021, 16:59 > RESOLVED in CODE WALKTHROUGH. (On behalf of Behrouz > NematiPour who is on vacation) ---------------------------------------- File: firmware/App/Modes/OperationModes.c Revision Comment by Behrouz NematiPour on 26 October 2021, 22:05 https://devapps.diality.us/cru/HD-DEN-9906-1#c10948 why here currentMode is casted while currentSubMode didn't? {code:C} data.opMode = (U32)currentMode; data.subMode = currentSubMode; {code} Reply by Sean Nash on 27 October 2021, 08:30 > currentSubMode is already a U32 - no cast needed. Reply by Behrouz NematiPour on 27 October 2021, 10:37 > RESOLVED IN CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Modes/Rinseback.c Revision Comment by hnguyen on 26 October 2021, 13:57 https://devapps.diality.us/cru/HD-DEN-9906-1#c10930 add last else else { } Reply by Sean Nash on 04 November 2021, 08:49 > There is nothing to be done in else, so why add it? Reply by Dara Navaei on 19 October 2023, 08:02 > RESOLVED in CODE WALKTHROUGH Revision Comment by hnguyen on 26 October 2021, 13:58 https://devapps.diality.us/cru/HD-DEN-9906-1#c10931 add last else else { } Reply by Sean Nash on 04 November 2021, 08:50 > There is nothing to be done in else, so why add it? Reply by Dara Navaei on 19 October 2023, 08:02 > RESOLVED in CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Modes/TreatmentStop.c Revision Comment by hnguyen on 26 October 2021, 14:01 https://devapps.diality.us/cru/HD-DEN-9906-1#c10932 should be if ( TRUE != getRinsebackCompleted() ) for consistency Reply by Sean Nash on 27 October 2021, 08:40 > If using a "==" comparison operator, I like to put the > literal first so that the compiler will error if I > accidentally use "=" instead. > Otherwise, I like to put the literal last because it reads > better. Reply by hnguyen on 27 October 2021, 10:44 > RESOLVED IN CODE WALKTHROUGH ---------------------------------------- File: MessageSupport.c Revision Comment by hnguyen on 26 October 2021, 12:59 https://devapps.diality.us/cru/HD-DEN-9906-1#c10916 Add file header Reply by Sean Nash on 26 October 2021, 14:31 > Fixed. Reply by hnguyen on 27 October 2021, 10:48 > RESOLVED IN CODE WALKTHROUGH ---------------------------------------- File: MessageSupport.h Revision Comment by hnguyen on 26 October 2021, 13:00 https://devapps.diality.us/cru/HD-DEN-9906-1#c10917 Add file header Reply by Sean Nash on 26 October 2021, 14:10 > Fixed. Reply by hnguyen on 27 October 2021, 10:48 > RESOLVED IN CODE WALKTHROUGH ---------------------------------------- File: firmware/App/Services/FPGA.c Revision Comment by Dara Navaei on 15 November 2021, 14:19 https://devapps.diality.us/cru/HD-DEN-9906-1#c11409 Why there are TBDs in the comments? Reply by Sean Nash on 15 November 2021, 14:55 > I wasn't sure what flow sensor "response" registers were - > moot now that sensors are gone. > Still unsure what to do with processor/pc error counts - will > resolve this in a future branch. Reply by Dara Navaei on 15 November 2021, 16:57 > RESOLVED in CODE WALKTHROUGH. ---------------------------------------- File: firmware/App/Services/CommBuffers.c Revision Comment by Behrouz NematiPour on 26 October 2021, 21:44 https://devapps.diality.us/cru/HD-DEN-9906-1#c10945 Does a toggle function/macro need the second and third parameters, since whatever value it currently has should be reversed? Reply by Sean Nash on 27 October 2021, 08:36 > This toggle macro is designed to be generic - so does not > assume you are toggling between 0 and 1. Could be toggling > between ON and OFF or OPEN and CLOSED, etc... So you have to > give the macro the two values that you are toggling between. Reply by Behrouz NematiPour on 27 October 2021, 10:39 > RESOLVED IN CODE WALKTHROUGH. --- ID: HD-DEN-9906-1 https://devapps.diality.us/cru/HD-DEN-9906-1 Title: HD-DEN-9906_HD Dev Flow Measurement Changes Statement of Objectives: State: Closed Summary: Author: Sean Nash Moderator: Sean Nash Reviewers: (1 active, 2 completed*) hnguyen (*) Dara Navaei (*) Behrouz NematiPour