This is a list of all comments for UI-DEN-5751-1. Review Summary: No summary ---------------------------------------- File: sources/gui/qml/pages/pretreatment/create/PreTreatmentBase.qml Revision Comment by plucia on 29 January 2021, 11:00 https://devapps.diality.us/cru/UI-DEN-5751-1#c7667 How will this notification bar look if an alarm is raised but minimized? Would there be two notification bars visible? Reply by Behrouz NematiPour on 01 February 2021, 14:02 > this notification bar shall not be confused with the alarm > notification bar. > - these are showing specific rejection reasons related to > each screen request. and resides on that one. Which may even > in some cases be covered by a higher priority alarm. > - Alarm bar is a general bar on the screen that shall always > be visible regardless of the screen we are currently in. > These two may exist at the same time and alarm may even cover > those rejection requests. Reply by plucia on 01 February 2021, 17:34 > RESOLVED ---------------------------------------- File: sources/gui/qml/pages/pretreatment/create/PreTreatmentCreate.qml Revision Comment by plucia on 29 January 2021, 11:05 https://devapps.diality.us/cru/UI-DEN-5751-1#c7670 Do these properties still exist? Reply by Behrouz NematiPour on 01 February 2021, 17:45 > created a sub-task for this as DoubleSlider doesn't have this > validation/active feature. > // TODO : Will be addressed in sub-task DEN-6686. Reply by plucia on 01 February 2021, 17:52 > will be addressed later, > RESOLVED Revision Comment by plucia on 29 January 2021, 11:02 https://devapps.diality.us/cru/UI-DEN-5751-1#c7668 If FW responds back saying either arterial or venous pressure limit ranges are invalid, the user corrects them, and then we proceed, without doing anything upon validation success the pressure limits would still show up as invalid the next time the user enters the create treatment screen. Reply by Behrouz NematiPour on 01 February 2021, 17:46 > created a sub-task for this as DoubleSlider doesn't have this > validation/active feature. > // TODO : Will be addressed in sub-task DEN-6686. Reply by plucia on 01 February 2021, 17:52 > will be addressed later, > RESOLVED ---------------------------------------- File: sources/gui/qml/components/ConfirmTreatmentSubTable.qml Revision Comment by plucia on 29 January 2021, 10:56 https://devapps.diality.us/cru/UI-DEN-5751-1#c7665 Can delete Reply by Behrouz NematiPour on 01 February 2021, 14:29 > Removed. Reply by plucia on 01 February 2021, 17:39 > RESOLVED ---------------------------------------- File: sources/gui/qml/globals/Variables.qml Revision Comment by plucia on 29 January 2021, 10:36 https://devapps.diality.us/cru/UI-DEN-5751-1#c7651 These limits should reside next to the other treatment parameter defaults in TreatmentCreate.h. Putting them in QML causes changes to the treatment parameters configuration file (specifically arterial and venous pressure limits) to have no effect. Reply by Behrouz NematiPour on 01 February 2021, 14:12 > Actually, this feature (Arterial/Venous Pressure) was done > before Pre-Treatment Create feature. > At the moment Pre-Treatment Create has been implemented > existence of these parameters has been ignored and another > approach has been followed. > In general, If a simple value can be defined in where it is > being used the most (in this case qml) it's better to be > defined there and not to call multiple setter, getter for > getting a simple *read only* value. Reply by plucia on 01 February 2021, 14:20 > Your solution requires a code change to be made and the > entire denali application to have to be rebuilt every time > a treatment parameter needs to be adjusted by systems. This > will not serve the needs of the system team. To fulfill > their needs it was requested at the time to put the values > in a file. You and I agreed at the time that the treatment > parameter ranges should be read from a file. These two > treatment parameter ranges are being read from QML which > contradicts our agreed-upon decision to have them read from > a file. Reply by Behrouz NematiPour on 01 February 2021, 14:23 > It is noted. > Since this is part of the Arterial/Venous pressure > feature and not the preTx_Uf, will be taking care of it > as part of the code clean-up and integration I need to do > later. > By the way, it's not my solution and not even my approved > one. Reply by plucia on 01 February 2021, 17:38 > RESOLVED Revision Comment by plucia on 29 January 2021, 10:40 https://devapps.diality.us/cru/UI-DEN-5751-1#c7653 here as well (see above) Reply by Behrouz NematiPour on 01 February 2021, 14:25 > See the reply above. Reply by plucia on 01 February 2021, 17:39 > RESOLVED ---------------------------------------- File: sources/gui/qml/pages/treatment/adjustments/TreatmentAdjustmentUltrafiltrationConfirm.qml Revision Comment by plucia on 29 January 2021, 10:57 https://devapps.diality.us/cru/UI-DEN-5751-1#c7666 It seems like some variables are being defined in QML while others are in cpp. I've seen this happen with TreatmentCreate. What's the plan for where PRS default variables should be defined? Reply by Behrouz NematiPour on 01 February 2021, 14:30 > if it is a *constant/read-only* value that *will be used > mostly in qml*, then better to be defined in QML unless > otherwise required. Reply by plucia on 01 February 2021, 17:35 > RESOLVED ---------------------------------------- File: sources/model/hd/alarm/MAlarmStatusData.cpp Revision Comment by plucia on 29 January 2021, 11:08 https://devapps.diality.us/cru/UI-DEN-5751-1#c7671 Can delete Reply by Behrouz NematiPour on 01 February 2021, 14:07 > Cannot find this line in the code. > May have been removed already. Reply by plucia on 01 February 2021, 17:46 > will be addressed later, > RESOLVED Revision Comment by pmontazemi on 29 January 2021, 17:08 https://devapps.diality.us/cru/UI-DEN-5751-1#c7673 Add missing function header. Reply by Behrouz NematiPour on 01 February 2021, 13:58 > The documentation has been managed in a way that this comment > will be inherited from the parent class MAbstract, virtual > function parameters(), so it doesn't need to be mentioned for > each child. > {code} > /*! > * \brief fromByteArray > * \details converts the values from Byte Arrays to the > Model data. > * \param vByteArray - the byte array input > * \param vIndex - current index of each data > section in byte array > * \return true on successful conversion > */ > virtual bool fromByteArray(const QByteArray > &vByteArray , int *vIndex = nullptr) = 0; > {code} Reply by pmontazemi on 01 February 2021, 14:15 > RESOLVED. ---------------------------------------- File: sources/model/hd/alarm/MAlarmTriggered.cpp Revision Comment by pmontazemi on 29 January 2021, 17:09 https://devapps.diality.us/cru/UI-DEN-5751-1#c7674 Add function missing header. Reply by Behrouz NematiPour on 01 February 2021, 13:55 > The documentation has been managed in a way that this comment > will be inherited from the parent class MAbstract, virtual > function parameters(), so it doesn't need to be mentioned for > each child. > {code} > /*! > * \brief parameters > * \return current data values of the models. > */ > virtual QVariantList parameters ( > ) const = 0; > {code} Reply by pmontazemi on 01 February 2021, 14:15 > RESOLVED. Revision Comment by pmontazemi on 29 January 2021, 17:09 https://devapps.diality.us/cru/UI-DEN-5751-1#c7675 Add function missing header. Reply by Behrouz NematiPour on 01 February 2021, 13:57 > The documentation has been managed in a way that this comment > will be inherited from the parent class MAbstract, virtual > function parameters(), so it doesn't need to be mentioned for > each child. > {code} > /*! > * \brief fromByteArray > * \details converts the values from Byte Arrays to the > Model data. > * \param vByteArray - the byte array input > * \param vIndex - current index of each data > section in byte array > * \return true on successful conversion > */ > virtual bool fromByteArray(const QByteArray > &vByteArray , int *vIndex = nullptr) = 0; > {code} Reply by pmontazemi on 01 February 2021, 14:14 > RESOLVED. ---------------------------------------- File: simulator/plugins/createtreatment/loader.py Revision Comment by plucia on 29 January 2021, 10:55 https://devapps.diality.us/cru/UI-DEN-5751-1#c7664 Should point to HDSimulator since denaliMessages has been deprecated Reply by Behrouz NematiPour on 01 February 2021, 14:08 > UI_DVT related. Reply by plucia on 01 February 2021, 14:15 > will be addressed later, > RESOLVED Revision Comment by plucia on 29 January 2021, 09:55 https://devapps.diality.us/cru/UI-DEN-5751-1#c7645 Should be {code} self.hd_simulator.set_treatment_heparin_data(volume) {code} Reply by Behrouz NematiPour on 01 February 2021, 14:09 > UI_DVT related. Reply by plucia on 01 February 2021, 17:45 > will be addressed later, > RESOLVED ---------------------------------------- File: simulator/plugins/alarms/loader.py Revision Comment by plucia on 29 January 2021, 10:18 https://devapps.diality.us/cru/UI-DEN-5751-1#c7650 This has been removed from dialin Reply by Behrouz NematiPour on 01 February 2021, 14:10 > UI_DVT related. Reply by plucia on 01 February 2021, 14:15 > will be addressed later, > RESOLVED Revision Comment by plucia on 29 January 2021, 10:03 https://devapps.diality.us/cru/UI-DEN-5751-1#c7646 Should be {code} self.hd_simulator.alarms.cmd_activate_alarm_id {code} Reply by Behrouz NematiPour on 01 February 2021, 14:09 > UI_DVT related. Reply by plucia on 01 February 2021, 17:45 > will be addressed later, > RESOLVED Revision Comment by plucia on 29 January 2021, 10:04 https://devapps.diality.us/cru/UI-DEN-5751-1#c7647 denaliMessages has been deprecated. Please don't add it back to dialin. This should be {code} self.hd_simulator.alarms.cmd_set_alarm_triggered {code} Reply by Behrouz NematiPour on 01 February 2021, 14:09 > UI_DVT related. Reply by plucia on 01 February 2021, 14:17 > will be addressed later, > RESOLVED Revision Comment by plucia on 29 January 2021, 10:07 https://devapps.diality.us/cru/UI-DEN-5751-1#c7649 denaliMessages has been deprecated. Please don't add it back to dialin. This should be {code} self.hd_simulator.alarms.cmd_set_alarm_cleared {code} Reply by Behrouz NematiPour on 01 February 2021, 14:10 > UI_DVT related. Reply by plucia on 01 February 2021, 14:17 > will be addressed later, > RESOLVED ---------------------------------------- File: simulator/plugins/inlinebloodpressures/loader.py Revision Comment by plucia on 29 January 2021, 10:55 https://devapps.diality.us/cru/UI-DEN-5751-1#c7662 Should point to HDSimulator, as denaliMessages has been deprecated Reply by Behrouz NematiPour on 01 February 2021, 14:08 > UI_DVT related. Reply by plucia on 01 February 2021, 17:44 > will be addressed later > RESOLVED ---------------------------------------- File: simulator/plugins/salinebolus/loader.py Revision Comment by plucia on 29 January 2021, 10:54 https://devapps.diality.us/cru/UI-DEN-5751-1#c7661 Should point to the HDSimulator Reply by Behrouz NematiPour on 01 February 2021, 14:08 > UI_DVT related. Reply by plucia on 01 February 2021, 14:16 > will be addressed later, > RESOLVED ---------------------------------------- File: simulator/plugins/treatmentranges/loader.py Revision Comment by plucia on 29 January 2021, 10:54 https://devapps.diality.us/cru/UI-DEN-5751-1#c7660 This should point to the HDSimulator to avoid future conflicts with dialin Reply by Behrouz NematiPour on 01 February 2021, 14:09 > UI_DVT related. Reply by plucia on 01 February 2021, 14:16 > will be addressed later, > RESOLVED ---------------------------------------- File: simulator/plugins/treatmentstates/loader.py Revision Comment by plucia on 29 January 2021, 10:53 https://devapps.diality.us/cru/UI-DEN-5751-1#c7659 Should point to the HDSimulator Reply by Behrouz NematiPour on 01 February 2021, 14:09 > UI_DVT related. Reply by plucia on 01 February 2021, 17:44 > will be addressed later, > RESOLVED ---------------------------------------- File: shared/scripts/names.py Revision Comment by pmontazemi on 29 January 2021, 17:17 https://devapps.diality.us/cru/UI-DEN-5751-1#c7679 What has caused this change in AUT_NAME? Reply by Behrouz NematiPour on 01 February 2021, 13:45 > This is related to two different AUT configuration in > SquishQt, > 1 - while running with build scripts locally to make sure for > the last time we have everything covered and that script > builds the app as "denaliSquish" on local and on the server > as well. > 2 - the other is when we are at the very first stage of the > testing and coverage and building back and forth to test and > cover. that one used the QtCreator built "denali". > To make it easy to work with these two configurations I made > the name of the AUT as a variable so we can switch between > quickly. > The last moment on pushing on the master has to have > "denaliSquish". Reply by pmontazemi on 01 February 2021, 14:14 > RESOLVED. ---------------------------------------- File: unittests/tst_messaging.cpp Revision Comment by pmontazemi on 29 January 2021, 17:12 https://devapps.diality.us/cru/UI-DEN-5751-1#c7678 Should we consider here the case where payload is not empty but has garbage values in it? Reply by Behrouz NematiPour on 01 February 2021, 13:51 > Sure, but that test is not here it is the checkCRC() function > test. Reply by pmontazemi on 01 February 2021, 14:14 > RESOLVED. --- ID: UI-DEN-5751-1 https://devapps.diality.us/cru/UI-DEN-5751-1 Title: UI-DEN-5751_Pre Treatment Ultrafiltration Statement of Objectives: Application : - Fixed comments in some files of the Pre-Treatment Create. - Removed not used SliderDoubleCreateTreatment.qml component. - A lot of code clean up and refactoring - cherry picked a fix from DEN-5830_AlarmDesign@51accb for the Treatment End message which has no payload. - Also made the interpretMessage() for received messages to be more identical to the DEN-5830_AlarmDesign to hopefully help merging easier. - Had a conversation with Syseng(Nic) and Fweng(Sean) and decided to have the fix somehow more understandable but there is no actual fix and if that happens it would be the FW bug. - The modification is that the main progress bar will always have VTreatmentUltratiltration.minimum as minimum (current;y always zero) and VTreatmentUltratiltration.maximum as maximum. - The UI always expects to get the vTreatmentRanges.treatmentRanges_Ultrafiltration_Volume_Min (adjustable min and slider min) greater than vTreatmentUltrafiltration.ultrafiltration_RefUFVol (current uf volume delivered and current value on the progress bar) and so close to each other. - Removed the precision from VTreatmentUltrafiltration and moved into the Variables. - linked the pre-treatment ultrafiltration adjustment with the in-treatment ultrafiltration section and the adjustment flow. - added the rejection reason text to the pTx uf adj screen. - Fixed the message id of the ID_ConfirmTreatmentReq from 58 to 59 - Enabled the priming button for now since there is no FW implementation. - Removed the unused flag of ALARM_ID_TEMPERATURE_SENSORS_INCONSISTENT - Removed the semi-colon at the end of the imports. - Added the new MVC for initial UF Volume adjustment messages (79/80). - reviewed the pre-treatment screens and modified - added the double range slider for Arterial and Venous - modified the treatmentBegin(ultrafiltration) - so many other tweaks. - reformat and commented out the message id debug log. TestSuits: - Reverted back the end treatment button click to cover that message. - Fixed ConfirmPrimingBegin - Fixed Create Treatment - added the message id 80 for pTx uf adj rsp. State: Closed Summary: Author: Behrouz NematiPour Reviewers: (0 active, 2 completed*) plucia (*) pmontazemi (*)