-
-
Notifications
You must be signed in to change notification settings - Fork 224
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP: Egeniouss refactore. #5706
base: master
Are you sure you want to change the base?
Conversation
🎉 Ta-daaa, freshly created APKs are available for aa8ced3: arm64-android |
3a4ee4d
to
8a0ffe9
Compare
instead of capturing connected and disconnected signals we can use stateChanged signal and simplify a bit.
8a0ffe9
to
d5277ec
Compare
override socketState and socketStateString from base class.
src/core/positioning/tcpreceiver.cpp
Outdated
emit socketStateChanged( socketState ); | ||
emit socketStateStringChanged( socketStateString() ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
like for egeniouss, could be replaced by a direct connection of the socket stateChanged with abstracGnss::stateChanged
src/core/positioning/tcpreceiver.cpp
Outdated
switch ( socketState ) | ||
QAbstractSocket::SocketState TcpReceiver::socketState() | ||
{ | ||
return mSocket ? mSocket->state() : QAbstractSocket::UnconnectedState; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here as wel mSocket doesn't need to be a pointer
src/core/positioning/tcpreceiver.cpp
Outdated
case QAbstractSocket::UnconnectedState: | ||
{ | ||
mSocketStateString = tr( "Disconnected" ); | ||
QString mSocketStateString = tr( "Disconnected" ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
QString mSocketStateString = tr( "Disconnected" ); | |
QString socketStateString = tr( "Disconnected" ); |
src/core/positioning/tcpreceiver.cpp
Outdated
@@ -122,6 +116,9 @@ void TcpReceiver::handleError( QAbstractSocket::SocketError error ) | |||
mLastError = tr( "TCP receiver error (%1)" ).arg( QMetaEnum::fromType<QAbstractSocket::SocketError>().valueToKey( error ) ); | |||
break; | |||
} | |||
|
|||
setSocketState( QAbstractSocket::UnconnectedState ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should not be there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For Udp the same apply as per Tcp and egeniouss
Thank you @domi4484 for your review. most of them addressed. |
4ba9e46
to
8ffc2b6
Compare
Remove duplicated codes of `socketState` and `socketStateString` also extract some logic from `socketStateString`.
1- Add header to abstractgnssreceiver.cpp, egeniousseeceiver.cpp, egeniousseeceiver.h. 2- Read-only socketState. 3- protected: `void setSocketState` 4- QBluetoothSocket::SocketState to QAbstractSocket::SocketState. 5- Pruning methods in sub classes.
PR Description
In this PR we are going to address these:
Step 1 of Refactor 1: Instead of capturing
connected
anddisconnected
signals we can usestateChanged
signal and simplify a bit.This pattern is been used in:
QField/src/core/positioning/bluetoothreceiver.cpp
Line 30 in 3a1586f
QField/src/core/positioning/tcpreceiver.cpp
Line 25 in 3a1586f
QField/src/core/positioning/udpreceiver.cpp
Line 38 in 3a1586f
Step 2 of Refactor 1: lets go for the bigger refactoring changing all of these.