This is a list of all comments for UI-DEN-7044-1. Review Summary: No summary ---------------------------------------- File: sources/view/VNetworkModel.cpp Revision Comment by Behrouz NematiPour on 29 March 2021, 09:15 https://devapps.diality.us/cru/UI-DEN-7044-1#c8842 This one might be on_ not do_. and should be private! A little confusing that if the signal comes from the interface how come the doAdd.... is public access? What happens if it has been called on the qml side. Could you please explain? Reply by plucia on 01 April 2021, 10:19 > I agree it should be private. I've updated it to onAddNetwork > and it is now private Reply by Behrouz NematiPour on 07 April 2021, 16:37 > RESOLVED Revision Comment by Behrouz NematiPour on 29 March 2021, 09:26 https://devapps.diality.us/cru/UI-DEN-7044-1#c8844 This is an exception that you have a view which you don't have access to its object but in general: *whoever wants to use it to connect to a signal of another class should connect to that signal in its own scop/class and do it with its private slots.* So in a case like this, these connections should be moved to the interface. But as I said it is an exception and there no better way than what has been done here. Reply by Behrouz NematiPour on 13 April 2021, 15:45 > RESOLVED Revision Comment by Behrouz NematiPour on 29 March 2021, 09:23 https://devapps.diality.us/cru/UI-DEN-7044-1#c8843 If you made sure your threading is fine please remove these debug which will fill up the log so fast. Please remove the other ones as well if not needed. Reply by plucia on 01 April 2021, 10:23 > Done Reply by Behrouz NematiPour on 07 April 2021, 16:36 > RESOLVED ---------------------------------------- File: sources/wifi/Network.h Revision Comment by pmontazemi on 24 March 2021, 12:46 https://devapps.diality.us/cru/UI-DEN-7044-1#c8728 Are these the only measurable levels? Reply by plucia on 31 March 2021, 16:44 > These are just thresholds by which to categorize the signal > strength of an AP. So, the actual measured signal strength > will vary independently of these levels. Reply by pmontazemi on 07 April 2021, 09:40 > RESOLVED. ---------------------------------------- File: sources/wifi/WifiInterface.cpp Revision Comment by Behrouz NematiPour on 29 March 2021, 09:35 https://devapps.diality.us/cru/UI-DEN-7044-1#c8846 Please take a closer look at the code. At this moment you are still in the main thread and not in the Wifi Thread. Reply by plucia on 01 April 2021, 10:48 > Moved this to onStart() and calling start() in main.cpp which > emits a private signal didStart() that is connected to > onStart as we discussed Reply by Behrouz NematiPour on 07 April 2021, 16:36 > RESOLVED ---------------------------------------- File: denali.pro Revision Comment by Behrouz NematiPour on 07 April 2021, 16:32 https://devapps.diality.us/cru/UI-DEN-7044-1#c9055 Please move it to line 139 Reply by plucia on 08 April 2021, 17:49 > Done Reply by Behrouz NematiPour on 13 April 2021, 15:38 > RESOLVED ---------------------------------------- File: sources/gui/GuiGlobals.cpp Revision Comment by Behrouz NematiPour on 29 March 2021, 10:25 https://devapps.diality.us/cru/UI-DEN-7044-1#c8847 Why Network being registered? Reply by plucia on 01 April 2021, 13:27 > It is now removed, WifiNetworkData is added to > REGISTER_MODEL_METATYPES now Reply by Behrouz NematiPour on 13 April 2021, 15:44 > RESOLVED ---------------------------------------- File: sources/gui/qml/pages/SettingsHome.qml Revision Comment by Behrouz NematiPour on 29 March 2021, 09:07 https://devapps.diality.us/cru/UI-DEN-7044-1#c8841 This screen is a child of ScreenItem and not a StackItem. How come the pop() function is working here! Which Push, pop methods is being called here? If that's the one from SettingsStack, would be more cleaner to move the push and pop functions in the SettingsStack. It's fine for now but keep that in mind for later. Reply by plucia on 01 April 2021, 10:06 > Updated it to use the settings stack specifically when > popping or pushing Reply by Behrouz NematiPour on 07 April 2021, 16:22 > RESOLVED ---------------------------------------- File: scripts/wifi_test_internet.sh Revision Comment by Behrouz NematiPour on 29 March 2021, 08:54 https://devapps.diality.us/cru/UI-DEN-7044-1#c8839 Would be better to use a non-profit website for the internet test. Reply by plucia on 31 March 2021, 17:04 > Done Reply by Behrouz NematiPour on 07 April 2021, 16:20 > RESOLVED ---------------------------------------- File: scripts/run.sh Revision Comment by Behrouz NematiPour on 29 March 2021, 08:42 https://devapps.diality.us/cru/UI-DEN-7044-1#c8838 This line caused some issues, we need to remove the log of the application output for now. please change it to by adding & and # before 2>>: $HOME/denali -u & # 2>> $HOME/filesystem.err & Reply by plucia on 31 March 2021, 16:47 > Done Reply by Behrouz NematiPour on 07 April 2021, 16:19 > RESOLVED ---------------------------------------- File: sources/gui/qml/dialogs/WifiJoinDisconnect.qml Revision Comment by Behrouz NematiPour on 29 March 2021, 09:03 https://devapps.diality.us/cru/UI-DEN-7044-1#c8840 Please Just make sure that the Alarm can show up and cover any dialog also. Reply by plucia on 01 April 2021, 09:57 > Just tested this and confirmed the alarm dialog shows up in > front of the join / disconnect dialog Reply by Behrouz NematiPour on 07 April 2021, 16:36 > RESOLVED ---------------------------------------- File: sources/model/MWifiNetwork.h Revision Comment by Behrouz NematiPour on 07 April 2021, 16:25 https://devapps.diality.us/cru/UI-DEN-7044-1#c9054 Thanks for doing this. But that is not what I meant. Struct Data should only have basic data types which require to be sending between threads. What is implemented here is an immediate struct definition in a class which will be passed and is literally the same as a class and has the only overhead of a struct defined in the class. Please use Data as the data and remove the functions from the struct Data. I think the concept of having the struct Data is not clarified, let me know if I need to explain mor. Reply by plucia on 08 April 2021, 17:45 > Addressed - ready for review Reply by Behrouz NematiPour on 13 April 2021, 15:39 > RESOLVED --- ID: UI-DEN-7044-1 https://devapps.diality.us/cru/UI-DEN-7044-1 Title: UI-DEN-7044_Wifi Statement of Objectives: State: Closed Summary: Author: plucia Moderator: plucia Reviewers: (0 active, 2 completed*) Behrouz NematiPour (*) pmontazemi (*)