This is a list of all comments for LEAHI-APPLICATION-LDT-1536-1. Review Summary: No summary General Comment by Stephen Quong on 12 August 2025, 11:01 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1536-1#c23628 Update SettingsWiFi.qml to not show contents of IP Address field when there is a WiFi connection error. General Comment by Nicholas Ramirez on 12 August 2025, 11:15 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1536-1#c23635 I pushed a fix for this behavior Reply by Stephen Quong on 12 August 2025, 11:16 > RESOLVED ---------------------------------------- File: sources/gui/qml/components/HeaderBar.qml Revision Comment by Behrouz NematiPour on 11 August 2025, 18:39 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1536-1#c23607 Please use IconButton and a simple implementation. This implementation is complicated and not flexible. Reply by Nicholas Ramirez on 12 August 2025, 09:21 > reverted back to original implementation to instantiate each > IconButton and remove repeater Reply by Behrouz NematiPour on 12 August 2025, 11:24 > RESOLVED Revision Comment by Behrouz NematiPour on 12 August 2025, 10:07 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1536-1#c23621 Please provide a description of this function. And please always add function documentation to your functions. Is this function help to locate the dialog under the associated IconButton? Reply by Nicholas Ramirez on 12 August 2025, 10:53 > added comments to function and moved to HeaderBarPopup Reply by Behrouz NematiPour on 12 August 2025, 11:02 > RESOLVE Revision Comment by Behrouz NematiPour on 01 August 2025, 16:40 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1536-1#c23460 ModalDialog definition should not be in the headerBar. Reply by Nicholas Ramirez on 11 August 2025, 09:23 > moved instantiating the dialog to main.qml Reply by Behrouz NematiPour on 12 August 2025, 11:25 > RESOLVED Revision Comment by Behrouz NematiPour on 01 August 2025, 16:40 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1536-1#c23461 Make this a component. Reply by Nicholas Ramirez on 04 August 2025, 14:12 > yes it is a component Reply by Behrouz NematiPour on 12 August 2025, 11:25 > RESOLVED Revision Comment by Behrouz NematiPour on 12 August 2025, 10:11 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1536-1#c23622 Don't you think it would be better to move the positioning to the HeaderBarPopup.qml and let that component to adjust itself with the sent Component.rectangle? Reply by Nicholas Ramirez on 12 August 2025, 10:52 > moved this logic in the HeaderBarPopup component Reply by Behrouz NematiPour on 12 August 2025, 11:24 > RESOLVED Revision Comment by Stephen Quong on 07 August 2025, 16:36 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1536-1#c23561 Remove semicolon. Reply by Nicholas Ramirez on 07 August 2025, 16:43 > removed Reply by Stephen Quong on 08 August 2025, 12:52 > RESOLVED ---------------------------------------- File: sources/gui/qml/components/ModalDialog.qml Revision Comment by Behrouz NematiPour on 01 August 2025, 16:39 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1536-1#c23459 v non-capital For these little dialogs, we do not want the dark overlay, though. Reply by Nicholas Ramirez on 04 August 2025, 14:13 > removed Reply by Behrouz NematiPour on 12 August 2025, 10:19 > thanks > RESOLVED Revision Comment by Stephen Quong on 12 August 2025, 11:15 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1536-1#c23634 Align color value with others. Reply by Nicholas Ramirez on 12 August 2025, 11:19 > good catch fixed alignement Reply by Stephen Quong on 12 August 2025, 11:22 > RESOLVED ---------------------------------------- File: sources/device/DeviceController.cpp Revision Comment by Behrouz NematiPour on 11 August 2025, 18:10 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1536-1#c23604 As we talked, please use check blocks instead of the nested multi-level if/else. Also, if you want to reuse the same variable, name it more generally, like info, instead of IP. {code} QString info = tr("WiFi Connection Error"); if ( ! _wifiValid ) { goto lOut; } info = tr("No WiFi"); if ( ! dev.isValid ) { goto lOut; } info = tr("WiFi Not Connected"); if ( ! addresses.count() ) { goto lOut; } if ( ! ( dev.flags().testFlag(QNetworkInterface::IsUp) && dev.flags().testFlag(QNetworkInterface::IsRunning) ) ) { goto lOut; } info = addresses.first().ip().toString(); lOut: emit didWifiData(info); {code} Reply by Nicholas Ramirez on 12 August 2025, 09:21 > updated to use check blocks instead Reply by Behrouz NematiPour on 12 August 2025, 09:47 > RESOLVED. Revision Comment by Behrouz NematiPour on 12 August 2025, 09:48 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1536-1#c23614 make the wlan0 a const private value of the class. Reply by Nicholas Ramirez on 12 August 2025, 09:59 > updated Reply by Behrouz NematiPour on 12 August 2025, 10:01 > Thank you [~nramirez], RESOLVED Revision Comment by Behrouz NematiPour on 12 August 2025, 09:46 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1536-1#c23612 Please use the lOut as I have mentioned above, and remove repeated emits. and move the definitions on top before the goto(s). Reply by Nicholas Ramirez on 12 August 2025, 09:59 > reverted to use the jumps Reply by Behrouz NematiPour on 12 August 2025, 10:02 > Thanks Nico! > RESOLVED. Revision Comment by Behrouz NematiPour on 11 August 2025, 18:19 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1536-1#c23605 I believe this should be in the response, so the situation that the script exists and can be executed, and is executed, and has some issues, is covered too. First line in the else block. Reply by Nicholas Ramirez on 12 August 2025, 09:20 > this haas been moved and renamed to _wifiAvailable Reply by Behrouz NematiPour on 12 August 2025, 09:51 > Even better naming, thanks. > RESOLVED. ---------------------------------------- File: sources/device/DeviceController.h Revision Comment by Behrouz NematiPour on 12 August 2025, 11:13 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1536-1#c23632 Please change this too, to WiFiIP as you did on the others. Reply by Nicholas Ramirez on 12 August 2025, 11:19 > updated this to be consistant Reply by Behrouz NematiPour on 12 August 2025, 11:22 > RESOLVED ---------------------------------------- File: sources/device/DeviceView.h Revision Comment by Behrouz NematiPour on 11 August 2025, 18:30 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1536-1#c23606 Please refactor all the ...WiFiData... in View, models, signal, slots, functions to something more specific like WiFiIP, since its sole purpose is to get the IP and no more data. Reply by Nicholas Ramirez on 12 August 2025, 09:21 > updated name to WiFiIP Reply by Behrouz NematiPour on 12 August 2025, 10:04 > RESOLVED. ---------------------------------------- File: sources/gui/qml/components/HeaderBarPopup.qml Revision Comment by Behrouz NematiPour on 12 August 2025, 10:58 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1536-1#c23627 Isn't it better if that is "white"? Can you try to show me the result? Reply by Nicholas Ramirez on 12 August 2025, 11:14 > yes i pushed the updated drop shadow color Reply by Behrouz NematiPour on 12 August 2025, 11:23 > RESOLVED ---------------------------------------- File: sources/gui/qml/globals/Colors.qml Revision Comment by Behrouz NematiPour on 12 August 2025, 11:05 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1536-1#c23630 I think this should be specific to some dialog, not all. likethe alarm or confirm. Please make this more specific. Reply by Nicholas Ramirez on 12 August 2025, 11:16 > this text is going to also be used with the Alarm list and > alarm dialogs Reply by Behrouz NematiPour on 12 August 2025, 11:23 > Got it! > Thanks! > RESOLVED ---------------------------------------- File: sources/gui/qml/main.qml Revision Comment by Behrouz NematiPour on 12 August 2025, 11:06 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1536-1#c23631 please remove the empty line. Reply by Nicholas Ramirez on 12 August 2025, 11:15 > removed Reply by Behrouz NematiPour on 12 August 2025, 11:22 > RESOLVED --- ID: LEAHI-APPLICATION-LDT-1536-1 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1536-1 Title: LEAHI-APPLICATION-LDT-1536_Feature/Ldt 1536 Headerbar Wifi Indicator Statement of Objectives: State: Closed Summary: Author: Nicholas Ramirez Moderator: Nicholas Ramirez Reviewers: (6 active, 2 completed*) Behrouz NematiPour (*) Stephen Quong (*) Sean Nash Tiffany Mejia Vinayakam Mani Michael Garthwaite Dara Navaei Daniel Ho