Skip to content
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

Core sensors framework #52297

Merged
merged 1 commit into from
Mar 24, 2023
Merged

Core sensors framework #52297

merged 1 commit into from
Mar 24, 2023

Conversation

nirvn
Copy link
Contributor

@nirvn nirvn commented Mar 19, 2023

Description

This PR implements a core sensors framework - a sensor types registry, a sensor manager - as well as three simple QIODevice-based sensor types (TCP socket, UDP socket, and serial port).

A sensor manager is attached project instances to allow for users to register sensors within their project files. Contrary to say a positioning device, I felt like sensors were much more project-specific. The big advantage of project-based sensors is that it makes those much more portable and easier to share across users.

The PR also adds a brand new expression sensor_data() function to the project scope, which returns the latest captured sensor data values for a specific sensor name. An optional expiration (in milliseconds) parameter allows for expressions that will reject a specific sensor value is older that the provided expiration value.

The expression function allows for symbology drawing that reflects sensors, e.g. this really dummy temperature sensor point coloring:

Peek.2023-03-19.11-59.mp4

The marker uses a data-defined expression for the fill color, and the layer is set to auto-repaint every 100ms

Having the expression function also allows for stuff like an expression-based default value for newly created features that would return captured sensor values.

The UI part to manage sensors will come up in a separate PR.

Sponsored by Sevenson Environmental Services.

@nirvn nirvn added the Feature label Mar 19, 2023
@github-actions github-actions bot added this to the 3.32.0 milestone Mar 19, 2023
@nirvn nirvn force-pushed the sensors_framework branch 2 times, most recently from 11566ce to 47c3116 Compare March 19, 2023 12:10
@nyalldawson
Copy link
Collaborator

src/core/sensor will need adding to https://github.com/qgis/QGIS/blob/master/doc/CMakeLists.txt#L50

python/core/auto_generated/sensor/qgssensorregistry.sip.in Outdated Show resolved Hide resolved
python/core/auto_generated/sensor/qgssensorregistry.sip.in Outdated Show resolved Hide resolved
src/core/qgsapplication.cpp Show resolved Hide resolved
tests/src/python/test_qgssensorregistry.py Show resolved Hide resolved
src/core/sensor/qgssensorregistry.h Show resolved Hide resolved
src/core/sensor/qgsiodevicesensor.h Show resolved Hide resolved
src/core/sensor/qgssensormanager.h Outdated Show resolved Hide resolved
src/core/sensor/qgssensormanager.h Show resolved Hide resolved
src/core/sensor/qgssensormanager.h Show resolved Hide resolved
resources/function_help/json/sensor_data Outdated Show resolved Hide resolved
@nirvn
Copy link
Contributor Author

nirvn commented Mar 20, 2023

@nyalldawson , thanks for the review, all comments have been addressed (code change and/or justification :) )

@nirvn nirvn added the API API improvement only, no visible user interface changes label Mar 20, 2023
@uclaros
Copy link
Contributor

uclaros commented Mar 20, 2023

Do you mind providing some more info on why this is to be included in core qgis, rather than a plugin?
From my (very) quick look it seems like it's addressing the needs of some quite specific project.

src/core/qgsapplication.h Show resolved Hide resolved
src/core/sensor/qgssensormanager.h Outdated Show resolved Hide resolved
src/core/sensor/qgsabstractsensor.h Outdated Show resolved Hide resolved
src/core/sensor/qgsabstractsensor.h Show resolved Hide resolved
@nirvn
Copy link
Contributor Author

nirvn commented Mar 21, 2023

@uclaros , sure, here are some points expanding on multiple uses of a sensors framework and its benefits in empowering QGIS ecosystem with it.

Capturing sensor data (wind, humidity, geologic compass, radiation detector instrument, etc.) while digitizing can be a crucial need for quite a few people here. Adding the framework into core means that QGIS’ overall ecosystem benefits: it directly benefits QIGS inbuilt GPS tools @nyalldawson has added some time ago as well as QField (which will incorporate the sensors framework), and leaves the door open for other QGIS downstream projects such as Mergin to make use of it in the future.

On top of fulfilling an important digitizing need, having the sensor framework within QGIS core means that you could have:

  • print layouts within which label items can reflect current state of sensors
  • map canvas with points representing sensors on the ground with a symbology that reflects the current state of those sensors
  • processing scripts, models, and algorithms making use of sensors data
  • etc.

As mentioned above, there is a follow up PR that will implement a GUI within QGIS to manage and read sensors on top of what’s being exposed via Python API.

A generic sensor framework design made to be expandable though python plugins is definitively not about some quite specific project. I’m looking forward to the many uses people will make of this (an MQTT plugin for example?).

@uclaros
Copy link
Contributor

uclaros commented Mar 21, 2023

It would make sense to me for qgis core to be able to consume sensor data using some standard like the ogc sensorthings api for example, but skipping the server part and interacting directly with individual sensors is kind of outside the GIS scope.
I'd really like to see other devs' opinions on this, as it still seems to me that this is plugin territory (would a QEP have been more appropriate?).

@nirvn
Copy link
Contributor Author

nirvn commented Mar 21, 2023

@uclaros , the sensors framework is built with that capability in mind. The fact that it has raw I/O device sensors to begin with isn’t prohibitive of having other sensor types that will access data throw other means such as REST APIs. Ideally, we’ll see an OGC Sensorthings sensor, a MQTT sensor, a <insert your favorite API, server, method> sensor, etc. :)

We do exactly the same thing when accessing e.g. NMEA strings through raw I/O channels – Bluetooth, virtual serial port on Windows, etc. - on the positioning front while we also support going through gpsd for example.

The raw I/O device sensors aren’t purely theoretical here, plenty of devices are using such a method to stream sensor values. Moreover, spatial software have had that capability for a long time too. Take for e.g. Trimble’s old TerrySync software, it had sensor capabilities that included serial port reading.

I can appreciate that you do not see needs in your workflows, but rest assured it is very much within the GIS scope :)

On the plugin angle, there is simply no way to get the necessary integration needed to achieve the examples I provided above without adding a framework in core. I am hoping to see interesting sensor-related QGIS plugins coming in the future, which this framework is also a prerequisite for.

@nyalldawson
Copy link
Collaborator

@uclaros

FWIW, I'm happy to see this introduced. I've been wanting to somehow extend the inbuilt GPS handling with support for non-position-related NMEA messages for a while (eg for devices which report air pressure/wind speed/...). It was always an open question on how well those non-position readings could fit into the QGIS GPS framework though... especially if you had an NMEA device which reported readings on a bunch of measurements but didn't have any GNSS capability at all! (eg a weather station)

@nirvn's approach looks sufficiently flexible to handle this use case, and the fact that it could be used in future by qfield/mergin is quite exciting. (I'd kinda love to make maps of noise levels just by walking around my local neighbourhood... )

src/core/qgis.h Outdated Show resolved Hide resolved
src/core/sensor/qgsabstractsensor.h Show resolved Hide resolved
src/core/sensor/qgsabstractsensor.h Outdated Show resolved Hide resolved
src/core/sensor/qgsabstractsensor.h Outdated Show resolved Hide resolved
src/core/sensor/qgsiodevicesensor.cpp Outdated Show resolved Hide resolved
src/core/sensor/qgsiodevicesensor.h Show resolved Hide resolved
@nirvn nirvn enabled auto-merge (rebase) March 24, 2023 03:40

void QgsTcpSocketSensor::setPort( int port )
{
if ( mPort == port || port < 1 )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't you also check if port > 65535? (also in udp code)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@uclaros , good point; I'll adjust in the other open PR as to avoid another 2 hours or CI (:sob:). Thanks for having had a look.

@nirvn nirvn merged commit d2b117a into qgis:master Mar 24, 2023
@m-kuhn m-kuhn added the Changelog Items that are queued to appear in the visual changelog - remove after harvesting label Jun 7, 2023
@qgis-bot
Copy link
Collaborator

qgis-bot commented Jun 7, 2023

@nirvn

This pull request has been tagged for the changelog.

  • The description will be harvested so please provide a "nearly-ready" text for the final changelog
  • If possible, add a nice illustration of the feature. Only the first one in the description will be harvested (GIF accepted as well)
  • If you can, it's better to give credits to your sponsor, see below for different formats.

You can edit the description.

Format available for credits
  • Funded by NAME
  • Funded by URL
  • Funded by NAME URL
  • Sponsored by NAME
  • Sponsored by URL
  • Sponsored by NAME URL

Thank you!

@zacharlie zacharlie added ChangelogHarvested This PR description has been harvested in the Changelog already. and removed Changelog Items that are queued to appear in the visual changelog - remove after harvesting labels Jun 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API API improvement only, no visible user interface changes ChangelogHarvested This PR description has been harvested in the Changelog already. Feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants