Add egeniouss receiver. #5637

mohsenD98 · 2024-09-10T16:08:45Z

Enable Egeniouss Receiver Support

Overview

This pull request implements support for Egeniouss receivers in the QField application. A new class, EgenioussReceiver, allows users to establish a TCP connection and interact with Egeniouss devices.

New Class: EgenioussReceiver

The EgenioussReceiver class, derived from AbstractGnssReceiver, has been created. It facilitates a TCP connection to Egeniouss devices, using a fixed host address (localhost) and port (1235).

Activation Instructions

The Egeniouss receiver is automatically enabled upon adding the Egeniouss plugin.

Additional Information

Within the plugin, we have two key functions: appWideEnabled() and appWideDisabled(). These functions are invoked in QField when the plugin is enabled or disabled. They simply toggle the egenioussEnabled property as follows:

plugin.positioningSettings.egenioussEnabled = true;  // To enable

or

plugin.positioningSettings.egenioussEnabled = false; // To disable

By utilizing egenioussEnabled within the positioningSettings, we can control the visibility of the EgenioussReceiver.

Download the Plugin

You can download the Egeniouss plugin here.

qfield-fairy · 2024-09-10T16:27:47Z

🎉 Ta-daaa, freshly created APKs are available for 7020594: arm64-android

Other architectures

assuming payload is a json. (right?)

No need for detailsChanged.

I hope sent utc is millisecond :)

Can we merge deviceChoosers ? TCP, UDP, Egeniouss? I'll revert QHostAddress::LocalHost later.

src/core/positioning/egenioussreceiver.h

src/core/positioning/egenioussreceiver.cpp

src/core/positioning/positioning.cpp

src/core/positioning/egenioussreceiver.h

Reproduce crash: 1- open app. 2- grant permissions. 3- app crashed. 4- reopen the app -- OK!

When we launch the app, it requests the internal receiver permission but then immediately change that to external, killing the object that called the permission.

src/qml/PluginManagerSettings.qml

Use plugin manager to call function attached to the plugin Item {} itself when enabling / disabling a plugin.

nirvn · 2024-09-19T16:22:05Z

src/core/pluginmanager.cpp

+  const QPointer<QObject> object = mLoadedPlugins[pluginPath];
+
+  const char *normalizedSignature = QMetaObject::normalizedSignature( ( methodName + "()" ).toStdString().c_str() );
+  const int methodExists = object->metaObject()->indexOfSlot( normalizedSignature );


methodIndex would be a better name. Or have methodExists be a bool and add >= 0 at the end of this line.

nirvn · 2024-09-19T16:22:48Z

@domi4484 , I'm good with this now. Do you want to have a look before we merge?

domi4484

Great thank you @mohsenD98!
I left some comments, @nirvn please take also a look maybe you don't agree on all of them :)

domi4484 · 2024-09-19T21:29:00Z

src/core/positioning/egenioussreceiver.h

I would move this class to an egeniouss directory inside positioning

@domi4484 , let's maybe not do this right away. ATM we have our other device classes sitting in positioning (Bluetooth, TCP, UDP, serial) here and having a directory for a single pair of .CPP, .H feels a bit too much.

If we end up needing another class (eg egenioussutils) let's maybe consider moving all of it into an src/core/egeniouss directory)

domi4484 · 2024-09-19T21:44:42Z

src/core/pluginmanager.cpp

@@ -288,10 +288,12 @@ void PluginManager::enableAppPlugin( const QString &uuid )
      loadPlugin( mAvailableAppPlugins[uuid].path(), mAvailableAppPlugins[uuid].name() );
    }
  }
+  callPluginMethod( uuid, "appWideActivated" );


Suggested change

callPluginMethod( uuid, "appWideActivated" );

callPluginMethod( uuid, "appWideEnabled" );

to reflect the method wording PluginManager::enableAppPlugin and be consistent with appWideDisabled

domi4484 · 2024-09-19T22:13:18Z

src/core/positioning/egenioussreceiver.cpp

+{
+  if ( mTcpSocket->state() == QAbstractSocket::ConnectedState )
+  {
+    mSocketState = QAbstractSocket::UnconnectedState;


It feels a bit strange to me that subclasses of postioning device have to manage mSocketState that way. mSocketState and mSocketStateString have a quite risk to get out of sync. Wouldn't be it a cleaner interface to reimplement AbstractGnssReceiver::socketState() and socketStateString in subclasses and get rid of the variables mSocketState and mSocketStateString?

@domi4484 , not a bad idea. Can we do that in a follow up PR?

src/core/positioning/egenioussreceiver.cpp

domi4484 · 2024-09-19T22:18:27Z

src/core/positioning/egenioussreceiver.cpp

+  switch ( error )
+  {
+    case QAbstractSocket::HostNotFoundError:
+      mLastError = tr( "Could not find the remote host" );


I guess with the current interface here you should set mSocketState and mSocketStateString as well

domi4484 · 2024-09-19T22:23:37Z

src/core/positioning/egenioussreceiver.cpp

+  }
+  if ( static_cast<uint8_t>( mReceivedData[0] ) != 0xFE )
+  {
+    return; // Invalid start byte


Here as well missing error handling

domi4484 · 2024-09-19T22:27:39Z

src/core/positioning/positioningdevicemodel.cpp

@@ -171,6 +171,9 @@ const QString PositioningDeviceModel::deviceId( const Device &device ) const

    case SerialPortDevice:
      return QStringLiteral( "serial:%1" ).arg( device.settings.value( QStringLiteral( "address" ) ).toString() );
+
+    case EgenioussDevice:
+      return QStringLiteral( "egeniouss:" );


I would put those prefixes into static const members. For serial:, udp:, and so on as well

Good idea, let's do that in a follow up PR though just to keep this manageable

domi4484 · 2024-09-19T22:29:20Z

src/core/positioning/tcpreceiver.cpp

@@ -119,7 +119,7 @@ void TcpReceiver::handleError( QAbstractSocket::SocketError error )
      mLastError = tr( "The connection was refused by the remote host" );
      break;
    default:
-      mLastError = tr( "UDP receiver error (%1)" ).arg( QMetaEnum::fromType<QAbstractSocket::SocketError>().valueToKey( error ) );
+      mLastError = tr( "TCP receiver error (%1)" ).arg( QMetaEnum::fromType<QAbstractSocket::SocketError>().valueToKey( error ) );


In this function also mSocketState and socket state string should be updated

Let's defer to different PR here to focus on egeniouss bits

domi4484 · 2024-09-19T22:31:23Z

src/qml/EgenioussDeviceChooser.qml

Would put this file into a egeniouss directory. Maybe into positioning/egeniouss and move as well other positioning related files in there to gain some structure

I suggest this structural change be kept in a separate PR

domi4484 · 2024-09-19T22:37:55Z

src/qml/PositioningSettings.qml

@@ -45,4 +45,5 @@ Settings {
  property int digitizingMeasureType: 1

  property bool geofencingPreventDigitizingDuringAlert: false
+  property bool enableEgeniouss: false


Suggested change

property bool enableEgeniouss: false

property bool egenioussEnabled: false

But for positioningActivated we use Active instead of Enable. But in the code we see generally more often "Enabled". Sot not sure which one you should choose 😄

Changes list: - Rename to `appWideEnabled` - Rename to `egenioussEnabled` - Set `mSocketStateString` and `mSocketState` - Use `const` variables in `onReadyRead`

setValid( true ) on constructor

Add egeniouss receiver.

aac7851

mohsenD98 self-assigned this Sep 10, 2024

mohsenD98 added 6 commits September 10, 2024 22:43

Select EgenioussDevice.

2f2e5e7

Refresh PositioningInformationModel on device details change.

f02339a

Fix ui and try to decode received message.

084823b

Read payload.

76963a1

assuming payload is a json. (right?)

Remove detailsChanged signal and use positionInformationChanged.

2009f87

No need for detailsChanged.

Set quality to 1 to pass mPositionInformation.isValid() condition.

7d9a9bc

mohsenD98 force-pushed the add-egeniouss-receiver branch from 1ca33bd to 7d9a9bc Compare September 13, 2024 07:40

mohsenD98 added 2 commits September 13, 2024 11:58

Try to set utc and fix crash on android.

089b1da

I hope sent utc is millisecond :)

Nanosecond utc + Connect address and port from ui.

ee635cf

Can we merge deviceChoosers ? TCP, UDP, Egeniouss? I'll revert QHostAddress::LocalHost later.

mohsenD98 force-pushed the add-egeniouss-receiver branch from a54ff80 to ee635cf Compare September 13, 2024 09:47

Try to use mAddress instead of localhost.

817566a

nirvn reviewed Sep 14, 2024

View reviewed changes

Address review - part 1.

be89cc5

m-kuhn reviewed Sep 14, 2024

View reviewed changes

src/core/positioning/egenioussreceiver.h Outdated Show resolved Hide resolved

mohsenD98 added 2 commits September 14, 2024 20:34

Address review - part 2.

661a623

[Try 1] to fix startup crash.

cd74cc9

Reproduce crash: 1- open app. 2- grant permissions. 3- app crashed. 4- reopen the app -- OK!

mohsenD98 closed this Sep 14, 2024

mohsenD98 reopened this Sep 14, 2024

mohsenD98 closed this Sep 15, 2024

mohsenD98 reopened this Sep 15, 2024

mohsenD98 added 6 commits September 15, 2024 16:33

[Try 2] Remove destructor.

deca732

[Try 3] add "this" to requestPermission.

65c6d8b

When we launch the app, it requests the internal receiver permission but then immediately change that to external, killing the object that called the permission.

Cleanup ui and some clean code.

7330347

Add egeniouss icon.

d30d81f

Fix style.

5c72442

Enable egeniouss receiver using plugin.

ea00df0

nirvn reviewed Sep 17, 2024

View reviewed changes

src/qml/PluginManagerSettings.qml Outdated Show resolved Hide resolved

Plugin manager signal when enabling / disabling plugin.

781d9f9

Use plugin manager to call function attached to the plugin Item {} itself when enabling / disabling a plugin.

mohsenD98 force-pushed the add-egeniouss-receiver branch from a6fcd04 to 781d9f9 Compare September 18, 2024 14:59

nirvn reviewed Sep 19, 2024

View reviewed changes

domi4484 reviewed Sep 19, 2024

View reviewed changes

mohsenD98 added 3 commits September 23, 2024 17:08

Address issues.

183fd28

Changes list: - Rename to `appWideEnabled` - Rename to `egenioussEnabled` - Set `mSocketStateString` and `mSocketState` - Use `const` variables in `onReadyRead`

Fix permission problem in android.

12f5ae5

Fix losing the positioning button on the lower right corner.

d05d013

setValid( true ) on constructor

mohsenD98 changed the title ~~WIP: Add egeniouss receiver.~~ Add egeniouss receiver. Sep 23, 2024

mohsenD98 marked this pull request as ready for review September 23, 2024 19:19

Fix style.

7020594

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add egeniouss receiver. #5637

Add egeniouss receiver. #5637

mohsenD98 commented Sep 10, 2024 •

edited

Loading

qfield-fairy commented Sep 10, 2024 •

edited

Loading

nirvn Sep 19, 2024

nirvn commented Sep 19, 2024

domi4484 left a comment

domi4484 Sep 19, 2024

nirvn Sep 23, 2024

domi4484 Sep 19, 2024

domi4484 Sep 19, 2024

nirvn Sep 23, 2024

domi4484 Sep 19, 2024

nirvn Sep 23, 2024

domi4484 Sep 19, 2024

domi4484 Sep 19, 2024

nirvn Sep 23, 2024 •

edited

Loading

domi4484 Sep 19, 2024

nirvn Sep 23, 2024

domi4484 Sep 19, 2024

nirvn Sep 23, 2024

domi4484 Sep 19, 2024

	callPluginMethod( uuid, "appWideActivated" );
	callPluginMethod( uuid, "appWideEnabled" );

	property bool enableEgeniouss: false
	property bool egenioussEnabled: false

Add egeniouss receiver. #5637

Are you sure you want to change the base?

Add egeniouss receiver. #5637

Conversation

mohsenD98 commented Sep 10, 2024 • edited Loading

Enable Egeniouss Receiver Support

Overview

New Class: EgenioussReceiver

Activation Instructions

Additional Information

Download the Plugin

qfield-fairy commented Sep 10, 2024 • edited Loading

Choose a reason for hiding this comment

nirvn commented Sep 19, 2024

domi4484 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nirvn Sep 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mohsenD98 commented Sep 10, 2024 •

edited

Loading

qfield-fairy commented Sep 10, 2024 •

edited

Loading

nirvn Sep 23, 2024 •

edited

Loading