This is a list of all comments for LEAHI-APPLICATION-LDT-1021-1. Review Summary: No summary ---------------------------------------- File: sources/gui/qml/components/ScreenItem.qml Revision Comment by Behrouz NematiPour on 23 June 2025, 18:37 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22245 Use standard keywords like: //FIXME : //TODO : to be listed in the QtCreator extension. Reply by Nicholas Ramirez on 24 June 2025, 08:17 > Updated and added keyword Reply by Behrouz NematiPour on 26 June 2025, 02:28 > RESOLVED ---------------------------------------- File: sources/gui/qml/components/StackItem.qml Revision Comment by Behrouz NematiPour on 23 June 2025, 18:38 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22246 Why were these two lines removed? By default, the stack item needs to fill in the parent. Reply by Nicholas Ramirez on 24 June 2025, 08:10 > The height was hard coded to a fixed number and since the > header bar sits on top of this the top portion of all the > settings pages were hidden behind Reply by Nicholas Ramirez on 24 June 2025, 16:21 > This has been reverted back to original Reply by Behrouz NematiPour on 26 June 2025, 02:28 > RESOLVED ---------------------------------------- File: sources/gui/qml/pages/settings/SettingsBase.qml Revision Comment by Behrouz NematiPour on 23 June 2025, 18:39 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22247 Why removed? Reply by Nicholas Ramirez on 24 June 2025, 08:09 > this was added as part of fixing the header bar sitting on > top of the settings pages as it was not needed anymore Reply by Nicholas Ramirez on 24 June 2025, 16:21 > this has been reverted back to original Reply by Behrouz NematiPour on 26 June 2025, 02:28 > RESOLVED ---------------------------------------- File: sources/device/DeviceController.cpp Revision Comment by Behrouz NematiPour on 23 June 2025, 18:34 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22244 The copyrights of the file will be automatically updated when it is built. Therefore, there is no need to update them manually. Reply by Nicholas Ramirez on 24 June 2025, 08:17 > update files that I edited this portion Reply by Behrouz NematiPour on 26 June 2025, 02:27 > RESOLVED Revision Comment by Behrouz NematiPour on 24 June 2025, 14:50 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22278 check if the code in the private slot is consistent with the rest of the code. Reply by Nicholas Ramirez on 24 June 2025, 16:11 > updated to be consistent Reply by Behrouz NematiPour on 26 June 2025, 02:15 > RESOLVED Revision Comment by Behrouz NematiPour on 29 June 2025, 16:36 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22498 Disconnecting +from+ Wifi. Additionally, please use +WiFi+ to maintain consistency with the message strings. Reply by Nicholas Ramirez on 01 July 2025, 08:38 > fixed Reply by Behrouz NematiPour on 01 July 2025, 16:57 > Please also fix the "Disconnecting *from* WiFi." Reply by Nicholas Ramirez on 03 July 2025, 10:41 > fixed string Reply by Behrouz NematiPour on 03 July 2025, 13:38 > RESOLVED ---------------------------------------- File: sources/device/DeviceController.h Revision Comment by Behrouz NematiPour on 24 June 2025, 14:48 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22277 remove this and will be done by the macros. Reply by Nicholas Ramirez on 24 June 2025, 16:11 > this has been removed Reply by Behrouz NematiPour on 26 June 2025, 02:16 > RESOLVED ---------------------------------------- File: sources/device/DeviceModels.cpp Revision Comment by Behrouz NematiPour on 24 June 2025, 14:43 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22275 put space. Reply by Nicholas Ramirez on 24 June 2025, 16:13 > Added missing space Reply by Behrouz NematiPour on 26 June 2025, 02:20 > RESOLVED ---------------------------------------- File: sources/device/DeviceModels.h Revision Comment by Behrouz NematiPour on 29 June 2025, 16:48 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22499 * Indent the fields. * Align the = Reply by Nicholas Ramirez on 01 July 2025, 08:39 > good catch. fixed Reply by Behrouz NematiPour on 01 July 2025, 16:54 > RESOLVED ---------------------------------------- File: sources/device/DeviceView.cpp Revision Comment by Behrouz NematiPour on 24 June 2025, 13:27 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22262 follow the WifiList model and attribute to avoid extra connection? Reply by Nicholas Ramirez on 24 June 2025, 16:13 > removed this connection Reply by Behrouz NematiPour on 26 June 2025, 02:29 > RESOLVED Revision Comment by Behrouz NematiPour on 24 June 2025, 14:32 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22274 remove these Reply by Nicholas Ramirez on 24 June 2025, 16:13 > these have been removed Reply by Behrouz NematiPour on 26 June 2025, 02:29 > RESOLVED Revision Comment by Behrouz NematiPour on 01 July 2025, 18:18 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22702 please fix the indent. Reply by Nicholas Ramirez on 03 July 2025, 10:42 > fixed indent Reply by Behrouz NematiPour on 03 July 2025, 13:37 > RESOLVED Revision Comment by Behrouz NematiPour on 29 June 2025, 16:50 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22500 Use *"// DEBUG: "* when commented out, a debug code that is kept for later use. Reply by Behrouz NematiPour on 01 July 2025, 16:12 > RESOLVED Revision Comment by Behrouz NematiPour on 29 June 2025, 16:51 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22501 Add one empty line before dataAppend. Reply by Behrouz NematiPour on 01 July 2025, 17:23 > RESOLVED Revision Comment by Behrouz NematiPour on 24 June 2025, 14:31 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22273 need to be removed. Reply by Nicholas Ramirez on 24 June 2025, 16:14 > removed this and moved to QML Reply by Behrouz NematiPour on 26 June 2025, 02:29 > RESOLVED Revision Comment by Behrouz NematiPour on 24 June 2025, 14:17 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22269 remove this and these will get updated in tehe response. Reply by Nicholas Ramirez on 24 June 2025, 16:14 > updated to move these to response Reply by Behrouz NematiPour on 26 June 2025, 02:30 > RESOLVED Revision Comment by Behrouz NematiPour on 24 June 2025, 14:22 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22270 move this to the private function wifiInfoRequest and call wifiInfoRequest here Reply by Nicholas Ramirez on 24 June 2025, 16:13 > updated and moved these into wifiInfoRequest() Reply by Behrouz NematiPour on 26 June 2025, 02:30 > RESOLVED Revision Comment by Behrouz NematiPour on 26 June 2025, 02:34 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22452 * If I remember correctly, we decided to assign the rest of the string to the DNS and manipulate it in the QML. * Generally, always use i++ in all for loops to maintain consistency. * Please consider using the mid function from the QStringList, which uses \n as a delimiter. Additionally, we can avoid using the "," since we are breaking into lines. {code} dns = fields.mid(eDNS).trimmed(); {code} *Please verify the proposed code for correctness.* Reply by Nicholas Ramirez on 26 June 2025, 17:06 > updated and tested Reply by Behrouz NematiPour on 29 June 2025, 17:58 > RESOLVED Revision Comment by Behrouz NematiPour on 24 June 2025, 14:12 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22267 make this one dns Reply by Nicholas Ramirez on 24 June 2025, 16:15 > updated to match the enum to the script return Reply by Behrouz NematiPour on 26 June 2025, 03:16 > RESOLVED Revision Comment by Behrouz NematiPour on 24 June 2025, 14:12 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22268 get the ssid from this response. Reply by Nicholas Ramirez on 24 June 2025, 16:15 > updated and added new READONLY for ssid Reply by Behrouz NematiPour on 26 June 2025, 03:11 > RESOLVED Revision Comment by Behrouz NematiPour on 24 June 2025, 14:23 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22271 take care of this when updated the enum Reply by Nicholas Ramirez on 24 June 2025, 16:17 > Update: I added some logic to append to the dns string if > there are more than one Reply by Behrouz NematiPour on 26 June 2025, 03:12 > RESOLVED Revision Comment by Behrouz NematiPour on 29 June 2025, 22:08 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22543 * Add the eCount * Check the size/count of the fields before use (before the if statement on line 559). +*It is always best practice to add an eCount enum item at the end of any enum and verify the size of the array or list that uses the enum as an index; otherwise, you are very likely to encounter an index out of range error and cause the software to crash.*+ *Please review the other enums you may have and apply the same.* Reply by Nicholas Ramirez on 01 July 2025, 08:36 > Updated and added check Reply by Behrouz NematiPour on 01 July 2025, 17:16 > RESOLVED. Revision Comment by Behrouz NematiPour on 01 July 2025, 17:02 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22699 If the script is not passing enough parameters, we need to notify the user. Use the {code} status (tr ("The WiFi info response error")); LOG_DEBUG(QString("The WiFi info response length is not correct [%1 of %2]").arg(len).arg(eCount)) {code} to notify user. +*Please test my suggested code/s*+ Reply by Nicholas Ramirez on 03 July 2025, 10:43 > Added notification to user Reply by Behrouz NematiPour on 03 July 2025, 13:33 > Please consider defining a variable for functions that are > used multiple times, like fields.size(), and then use that > value afterwards. > But for now, > RESOLVED. Revision Comment by Behrouz NematiPour on 26 June 2025, 03:15 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22461 The basic types like bool, int, ... do not need to be const &, and this improvement mainly affects classes. Reply by Nicholas Ramirez on 26 June 2025, 17:05 > fixed Reply by Behrouz NematiPour on 29 June 2025, 17:36 > RESOLVED Revision Comment by Behrouz NematiPour on 29 June 2025, 18:00 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22505 The trimmed() needs to be done before the join, otherwise it will not take effect on the joined fields. Reply by Nicholas Ramirez on 01 July 2025, 08:38 > updated Reply by Behrouz NematiPour on 01 July 2025, 17:01 > RESOLVED Revision Comment by Behrouz NematiPour on 01 July 2025, 16:59 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22697 Not necessary to trim them all. Just leave it to the script, this is informational. Reply by Nicholas Ramirez on 03 July 2025, 11:29 > updated and removed for now Reply by Behrouz NematiPour on 03 July 2025, 13:35 > RESOLVED Revision Comment by Behrouz NematiPour on 26 June 2025, 02:55 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22453 Please always use spaces before and after the not ( "!" ) character. Reply by Nicholas Ramirez on 26 June 2025, 17:05 > fixed Reply by Behrouz NematiPour on 29 June 2025, 18:09 > RESOLVED Revision Comment by Behrouz NematiPour on 26 June 2025, 03:01 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22455 I don't think this is being used correctly. Since this is an attribute, not a property, it entails more to call the script as we discussed. Same on line 574. Reply by Nicholas Ramirez on 26 June 2025, 17:05 > removed Reply by Behrouz NematiPour on 03 July 2025, 13:39 > RESOLVED Revision Comment by Behrouz NematiPour on 26 June 2025, 03:04 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22456 * I believe we discussed calling the WifiInfoRequest to update the left section of the WiFi screen. Therefore, the if block starting from line 577 should be removed. * And also discussed adding functionality to set the successfully connected SSID highlighted in the vDeviceView and avoid recalling the wifiListRequest. Reply by Nicholas Ramirez on 26 June 2025, 17:05 > updated Reply by Behrouz NematiPour on 03 July 2025, 13:38 > RESOLVED Revision Comment by Behrouz NematiPour on 26 June 2025, 10:10 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22464 1) remove the doInintWifiList 2) call the wifiInforequest 3) add a function to highlight the active connection(empty or not) 4) The function in step 3 needs to be called in the call of step 2. Reply by Nicholas Ramirez on 26 June 2025, 17:05 > updated and now added a overwridden setData() method to > update the QAbstractListModel and emit the changes so the qml > updates accordingly Reply by Behrouz NematiPour on 03 July 2025, 13:38 > RESOLVED ---------------------------------------- File: sources/device/DeviceView.h Revision Comment by Behrouz NematiPour on 29 June 2025, 16:03 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22496 These four rules, along with the rest of the references to them, need to be removed (Not Used). Reply by Nicholas Ramirez on 01 July 2025, 08:39 > removed Reply by Behrouz NematiPour on 01 July 2025, 13:30 > RESOLVED Revision Comment by Behrouz NematiPour on 26 June 2025, 03:09 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22457 Please keep the attributes close together and add an empty line between those and the rest of the normal PROPERTY/READONLY properties. {code} ATTRIBUTE ( QStringList , wifiList , {} , WifiList ) ATTRIBUTE ( QStringList , wifiInfo , {} , WifiInfo ) ATTRIBUTE ( bool , wifiConnect , false, WifiConnect ) READONLY ( bool , wifiListEnabled , true ) READONLY ( QString , ssid , "" ) READONLY ( QString , ipAddress , "" ) READONLY ( QString , gateway , "" ) READONLY ( QString , subnetMask , "" ) READONLY ( QString , dns , "" ) {code} Reply by Nicholas Ramirez on 03 July 2025, 10:41 > updated and organized Reply by Behrouz NematiPour on 03 July 2025, 13:35 > RESOLVED Revision Comment by Behrouz NematiPour on 24 June 2025, 14:05 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22266 remove these and use the doInit is. Reply by Nicholas Ramirez on 24 June 2025, 16:20 > removed this method Reply by Behrouz NematiPour on 26 June 2025, 03:11 > RESOLVED ---------------------------------------- File: sources/gui/qml/pages/settings/SettingsWiFi.qml Revision Comment by Behrouz NematiPour on 26 June 2025, 02:24 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22439 Please keep the ipValidator, to be consistent, and in case later need to reuse the entries for the static. Reply by Behrouz NematiPour on 29 June 2025, 17:57 > RESOLVED Revision Comment by Behrouz NematiPour on 24 June 2025, 13:53 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22264 add the doInitWifiInfo and remove thsi to be the returned result of the call. Reply by Nicholas Ramirez on 24 June 2025, 16:22 > this has been updated to use doInitWifiInfo Reply by Behrouz NematiPour on 26 June 2025, 02:25 > RESOLVED Revision Comment by Behrouz NematiPour on 24 June 2025, 14:01 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22265 remove this line, and we need to have a binding on the ssid role from vdevice on the ssid test. Reply by Nicholas Ramirez on 24 June 2025, 16:21 > update to bind ssid to this and moved location at the top of > the info list Reply by Behrouz NematiPour on 26 June 2025, 02:25 > RESOLVED Revision Comment by Behrouz NematiPour on 24 June 2025, 13:52 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22263 remove this and act upon the response of the doConnectWifi Reply by Nicholas Ramirez on 24 June 2025, 16:22 > this connection has been removed Reply by Behrouz NematiPour on 26 June 2025, 02:26 > RESOLVED ---------------------------------------- File: sources/device/DeviceError.cpp Revision Comment by Behrouz NematiPour on 29 June 2025, 16:17 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22497 * Please update the two messages to start with "The WiFi ..." as the other ones do. * Align the prantessis. Reply by Nicholas Ramirez on 01 July 2025, 08:39 > updated Reply by Behrouz NematiPour on 01 July 2025, 16:54 > RESOLVED ---------------------------------------- File: sources/device/DeviceError.h Revision Comment by Behrouz NematiPour on 24 June 2025, 14:46 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22276 add another for the disconnect and identify the error in the relative request of the response. Reply by Nicholas Ramirez on 24 June 2025, 16:12 > added another error for disconnect Reply by Behrouz NematiPour on 26 June 2025, 02:18 > RESOLVED Revision Comment by Behrouz NematiPour on 26 June 2025, 03:18 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22463 Please ensure that all ConnectWifi and DisconnectWifi (enum, function, class, function, error string, macro usage, etc.) functions use the 'wifi' as a prefix, like the others, to maintain consistency. Like: ...WifiList... ...WifiInfo... ...WifiConnect... ...WifiDisconnect... Please, * If it starts with wifi, then use non-capital 'w', like properties. * If in the middle, then use capital 'W' *This change needs to be done in the entire Device module (MVC,qml)* . Reply by Nicholas Ramirez on 26 June 2025, 17:05 > updated Reply by Behrouz NematiPour on 29 June 2025, 18:11 > RESOLVED ---------------------------------------- File: sources/gui/qml/main.qml Revision Comment by Behrouz NematiPour on 23 June 2025, 18:41 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22248 Do not anchor to the headerbar. Reply by Nicholas Ramirez on 24 June 2025, 08:07 > i added this to fix the headerbar overlaying on all the > settings pages. Since the headerbar was just place on top the > buttons and titles were hidden behind it. Reply by Nicholas Ramirez on 24 June 2025, 16:22 > this has been reverted and added a FIXME instead to address > later Reply by Behrouz NematiPour on 26 June 2025, 02:20 > RESOLVED Revision Comment by Behrouz NematiPour on 24 June 2025, 14:59 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1#c22279 revert the headerBar anchoring anywhere in this code and leave FIXME: BEHROUZ so I can fix it later. --- ID: LEAHI-APPLICATION-LDT-1021-1 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1021-1 Title: LDT-1021 - Device Settings - SW - 01 - Wifi - R&I - Application Statement of Objectives: State: Closed Summary: Author: Nicholas Ramirez Moderator: Nicholas Ramirez Reviewers: (5 active, 2 completed*) Tiffany Mejia (*) Behrouz NematiPour (*) Sean Nash Vinayakam Mani Michael Garthwaite Dara Navaei Daniel Ho