This is a list of all comments for UI-DEN-10205-1. Review Summary: No summary ---------------------------------------- File: sources/bluetooth/BluetoothInterface.cpp Revision Comment by Michael Garthwaite on 21 December 2021, 10:14 https://devapps.diality.us/cru/UI-DEN-10205-1#c11556 Out of curiosity, could you explain the difference between what #include go into the header file to those that are #include in the source file? Edit: I understand that BluetoothInterface.h is needed, but what about the others? Reply by Behrouz NematiPour on 21 December 2021, 13:05 > That is a very good question. > As much as possible we have to put the #includes in the CPP > files and not the header. > The #include in the header will always be imported even if > you don't compile the code (only CPP is compiled in c, not > the header) as opposed to just including when needed to > compile in CPP. > This also helps the compiler to optimize the code better and > faster to compile. > you only need to put it in the header if you inherit from the > class defined in the class included or you have a > variable/parameter defined of that file in the header. Reply by Michael Garthwaite on 21 December 2021, 13:27 > Thanks. Reply by Michael Garthwaite on 21 December 2021, 13:27 > RESOLVED Revision Comment by Michael Garthwaite on 21 December 2021, 10:07 https://devapps.diality.us/cru/UI-DEN-10205-1#c11555 This is calling didAttributeResponse from DeviceGlobals.h, correct? Reply by Behrouz NematiPour on 21 December 2021, 14:04 > A very important concept. > I refer you to the Qt connection concept for detailed > information. > https://doc.qt.io/qt-5/signalsandslots.html > The idea is, the connect has 4 parameter > 1 - Signal owner object > 2 - Signal member function > 3 - Slot owner object > 4 - Slot member function > When the signal owner is emit-ed, the slot owner will call > its slot by the passed parameter from the signal function. > It is very important to use the connection concept, in > opposed to the simple call, in case the signal owner and slot > owner, objects, are in two different threads. > It is the Qt way of thread-safe calling. Reply by Michael Garthwaite on 22 December 2021, 12:56 > Thank you! Reply by Michael Garthwaite on 22 December 2021, 12:56 > RESOLVED Revision Comment by Dara Navaei on 21 December 2021, 13:41 https://devapps.diality.us/cru/UI-DEN-10205-1#c11568 Do you still need this case? Reply by Behrouz NematiPour on 21 December 2021, 14:19 > I would rather keep it for debugging purposes in case of > error. Reply by Dara Navaei on 03 January 2022, 13:56 > In the firmware we add a TODO to be able to grep them > easily and make sure we do not forget any commented code. > RESOLVED. ---------------------------------------- File: sources/bluetooth/BluetoothInterface.h Revision Comment by Dara Navaei on 21 December 2021, 14:21 https://devapps.diality.us/cru/UI-DEN-10205-1#c11571 I understand that the friend classes are for dev testing, are we planning to disable them later during the release? Reply by Behrouz NematiPour on 22 December 2021, 11:10 > Ummm, > Haven't ever thought about it, this is a very good point. > Will look into it, since the actual friend class will not > ever be available in the production code. Reply by Dara Navaei on 03 January 2022, 13:56 > RESOLVED. ---------------------------------------- File: sources/gui/qml/pages/treatment/sections/TreatmentHeparin.qml Revision Comment by Michael Garthwaite on 21 December 2021, 11:19 https://devapps.diality.us/cru/UI-DEN-10205-1#c11558 With Off and Complete being two different properties, should we have Complete say "Off" to the user and not "Complete?" Reply by Behrouz NematiPour on 21 December 2021, 13:02 > These changes have been requested by SysEngz. > Let them have a look and decide if it's what they want. Reply by Michael Garthwaite on 22 December 2021, 12:57 > RESOLVED ---------------------------------------- File: sources/device/DeviceError.cpp Revision Comment by Dara Navaei on 21 December 2021, 14:35 https://devapps.diality.us/cru/UI-DEN-10205-1#c11573 Why is vScript not a const? Reply by Behrouz NematiPour on 22 December 2021, 11:03 > The vScript parameter will accept the arguments which will be > changed in the function, so has been defined as a reference > with no const. Reply by Dara Navaei on 03 January 2022, 13:53 > RESOLVED. ---------------------------------------- File: scripts/bluetooth_paired_query.sh Revision Comment by Michael Garthwaite on 21 December 2021, 09:35 https://devapps.diality.us/cru/UI-DEN-10205-1#c11554 is $PAIRED_DEVICE_INFO an environment variable? Reply by Behrouz NematiPour on 22 December 2021, 10:59 > it has been fixed on the merge conflict. > 748e088be863e08a1f8d12ff895c2525cd891e31 Reply by Michael Garthwaite on 22 December 2021, 12:56 > RESOLVED ---------------------------------------- File: sources/device/DeviceController.cpp Revision Comment by Dara Navaei on 21 December 2021, 14:32 https://devapps.diality.us/cru/UI-DEN-10205-1#c11572 Why does not this function have an argument name? Reply by Behrouz NematiPour on 22 December 2021, 11:07 > if the parameter is not used, the name will not be specified > so the compiler won't complain about the parameter not being > used. > > Note the difference between parameters and arguments: > - Function parameters are the names listed in the function's > definition. > - Function arguments are the real values passed to the > function. Reply by Dara Navaei on 03 January 2022, 13:55 > Sorry I meant parameters not arguments. Reply by Dara Navaei on 03 January 2022, 13:55 > RESOLVED. Revision Comment by Sean Nash on 29 November 2021, 08:55 https://devapps.diality.us/cru/UI-DEN-10205-1#c11458 Many of these methods do not have headers. Reply by Behrouz NematiPour on 30 November 2021, 16:27 > These functions are defined by macros and have the comment > headers in macro and it has been set up in oxygen to grab the > comments from the defined macro and not the implementation. > So long story short, the comments for these methods > onAttributeRequest & onProcessBluetoothPairedResetExitCode, > will be inherited from macros. > They are all overloaded function members (same name different > signature) Reply by Sean Nash on 09 December 2021, 09:44 > RESOLVED. ---------------------------------------- File: tst_HomeScreen/test.py Revision Comment by Michael Garthwaite on 21 December 2021, 10:25 https://devapps.diality.us/cru/UI-DEN-10205-1#c11557 This file was changed but no changes are highlighted, any idea what happened? Reply by Behrouz NematiPour on 21 December 2021, 13:04 > Sometimes it's only the FS properties (changed to executable) > of the file that git takes it as a change. Reply by Michael Garthwaite on 21 December 2021, 13:29 > RESOLVED --- ID: UI-DEN-10205-1 https://devapps.diality.us/cru/UI-DEN-10205-1 Title: UI-DEN-10205_BCuff Missing Features Reconnect Paired Statement of Objectives: State: Closed Summary: Author: Behrouz NematiPour Moderator: Behrouz NematiPour Reviewers: (0 active, 3 completed*) Sean Nash (*) Michael Garthwaite (*) Dara Navaei (*)