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

Improve Parsing of Data from Serial Port Sensors #56275

Closed
jtornero opened this issue Feb 9, 2024 · 4 comments
Closed

Improve Parsing of Data from Serial Port Sensors #56275

jtornero opened this issue Feb 9, 2024 · 4 comments

Comments

@jtornero
Copy link
Contributor

jtornero commented Feb 9, 2024

Feature description

This will make possible for the serial port sensors to parse whole sentences instead of splitting the data in random chunks with the current implementation, so further processing of the sensor's data is easier (No need for rejoining the data and search delimiters, etc)

Please consider if this could be treated as a bug just in case it shoul require additional actions from me to report the issue.

Additional context

The Project->Sensors features makes possible connecting QGIS to a wide range of data sources, as the user can choose between TCP, UDP and serial port connections.

However, the behavior of the serial port sensors is not optimal. It seems that it arbitrarily splits the incoming data in somehow random chuncks so it becomes difficult to process, providing lots of data come in the form of sentences, lines, datagrams or similar structures delimited by a special character (\n usually). Therefore the user must provide methods for processing the incoming data so the sentences are properly captured. This screencast shows the current behavior:

serialsensor_badparsing.mp4

In its current implementation, the feature depends on the method QgsIODeviceSensor::parseData(), which is inherited by the three sensor classes (QgsTcpSocketSensor, QgsUDPSocketSensor and QgsSerialPortSensor):

void QgsIODeviceSensor::parseData()
{
  QgsAbstractSensor::SensorData data;
  data.lastValue = mIODevice->readAll();
  data.lastTimestamp = QDateTime::currentDateTime();
  setData( data );
}

It turns out that for serial devices is not possible for the readAll() method to know when a sentence ends, so it reads data up to a certain point (internal buffers, timeouts...?) so sentences are split. In fact, testing the serial port connection with different baud rates shows that the faster the connection, the more data is captured by the sensor, i.e. from just a few characters as seen in the screencast (4800 baud) to a whole sentence at 115200 baud.

In the case of the serial ports is widely advised the use of the method readLine() and canReadLine() to get the data properly. I've tested a simple implementation of the idea and it works properly, just overriding the method parseData() for the class QgsSerialPortSensor with the use of readLine(), etc:

void QgsSerialPortSensor::parseData()
{

  if ( mSerialPort->canReadLine( ) )
  {
    QgsAbstractSensor::SensorData data;
    data.lastValue = mSerialPort->readLine();
    data.lastTimestamp = QDateTime::currentDateTime();
    setData( data );
  }
}

With this first, simple approach, serial data can be parsed properly as can be seen here:

serialsensor_parsingok.mp4

However, two issues arise:

  • I'm not aware of true serial data providers, i.e., sensors which send a fixed amount of data through the serial port or without a sentence delimiter. In this case, maybe the behavior of the sensor should be selectable.
  • Should the user be given the option to select the statement delimiter? In this case, `readLine() is not possible because it only searches for \n delimiter, therefore a different approach must be used.

It would be nice if someone could clarify these two issues so I could tackle this issue myself.

Thanks a lot

@jtornero jtornero changed the title Proper parsing of data from Serial port Sensors Improve Parsing of Data from Serial Port Sensors Feb 9, 2024
@nirvn
Copy link
Contributor

nirvn commented Feb 20, 2024

@jtornero , I think the solution here would be to allow for users to define a delimiter.

@jtornero
Copy link
Contributor Author

@jtornero , I think the solution here would be to allow for users to define a delimiter.

Well, then I guess the best solution should be having an editable combobox with two harcoded values (No delimiter plus \n) just for having some default, fast setting values, so the user could set a custom delimiter as well.

Or maybe a radiobutton group with 'No delimiter', '\n' and 'Custom' with a Lineedit could do the trick.

Is there any preferred GUI widgets in QGIS for this kind of things?

Cheers

@jtornero
Copy link
Contributor Author

jtornero commented Mar 1, 2024

Hello,

While developing the feature I got stuck at some point while trying the implementation of the custom delimiter for the data. Newline splitting, however, was ok as you can see in this video

vokoscreenNG-2024-03-01_21-43-50.mp4

But the other days for some reason I had to use another sensor simulator, this time based on a Raspberry Pi Pico instead of arduino... and the no-delimiter parsing (the original) worked like a charm. That surprised me a little, to be honest (previous test were both with the arduino simulator and a serial GPS with FDTI convertor.

I did some digging and ran a few more tests with different setups—pure-serial devices, various USB-to-serial converters. Turns out, it's not QGIS parsing causing the problem; it's the devices themselves.

Adittionally, in some scenarios a side effect of splitting the data fixing a delimiter arises. Some devices send several sentences of data at once, so if you split the data with a delimiter what you get in sensor.data.lastValue is the last parsed sentence, not the whole thing. For, say so, high frequency sensors could be a somehow nice feature but for sensors which send different information in a group of sentences won't be ok.

So, at the end, taking into account the risk of losing information and that the user has to parse/proccess further the lastValue in most scenarios (multi-sentence streams, for instance) maybe implementing the delimiter splitting is not a great idea.

So maybe we should left the things in its current state. In that case, I think it would be nice improving the documentation relative to serial port sensors including an advice (with a sample of code included) about how to process and parse the data and also how to manage sentence fragmentation in plugins, scripts, etc.

What do you think about this?

Finally, I don't know it the code I've generated so far could be useful in other any way, please let me know.

Cheers

Jorge

@jtornero
Copy link
Contributor Author

This issue has been been solved and the feature implemented by @nirvn with PR #56779
Thank you very much for all

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants