- settings are only applied to the element selected first in the project explorer
- add name (editable) and source (read-only) line edit
- optimize access to MQTT host data (reduce QMap searches)
- disable/enable reading type options for different source types
- distinguish MQTT source by host name and port
- allow different LiveDataSources from the same host
Details
Details
- Reviewers
asemke - Group Reviewers
LabPlot - Commits
- R262:f196ea9603ff: Improve LiveDataDock and import widget
- create new LiveData sources (Ascii, ROOT, MQTT)
- select several of them in the project explorer, but edit only one
- change name in dock widget
Diff Detail
Diff Detail
- Repository
- R262 LabPlot
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Comment Actions
The dock looks like this:
| src/kdefrontend/dockwidgets/LiveDataDock.cpp | ||
|---|---|---|
| 1245–1246 | Actually I'm not so sure, what m_previousMQTTClient (formerly m_clients[m_previousMQTTClient->clientHostName()]) is supposed to do. Could someone clarify? | |
| src/kdefrontend/GuiObserver.cpp | ||
|---|---|---|
| 573 | why not to simply auto* client = qobject_cast<MQTTClient*>(selectedAspects.first()); m_mainWindow->m_liveDataDock->setMQTTClient(cleint) ? | |
| src/kdefrontend/dockwidgets/LiveDataDock.cpp | ||
|---|---|---|
| 396 | m_mqttClient is only defined if HAVE_MQTT | |
Comment Actions
So would you agree, that qobject_cast is not required, since the class name is checked anyway?
The result of the cast isn't checked anywhere, so one could just use static_cast (and simply take the first element in the list).
| src/kdefrontend/dockwidgets/LiveDataDock.cpp | ||
|---|---|---|
| 396 | Thanks, of course. | |
| src/kdefrontend/MainWin.cpp | ||
|---|---|---|
| 1923 | this piece of code you removed now made sure we don't connect to the same broker twice. Here we also showed an message to the user to make this clear. To me it make sense. We should maybe adapt this logic and extend it to other data sources, too. Was there any reason to remove this logic? You partially moved this logic to LiveDataDock::setMQTTClient(), right? | |
| src/kdefrontend/MainWin.cpp | ||
|---|---|---|
| 1923 | Initially I removed this, because there was no check for the port, which makes this check exclude more hosts than required, but then I added that logic in the dock widget. On the other hand, that should not be part of this patch. So I will take it back in (including a check for the port number). | |
| src/kdefrontend/dockwidgets/LiveDataDock.cpp | ||
|---|---|---|
| 1245–1246 | I asked Ferencz, who contributed this code. The idea here was disconnect from the slot if we are going to connect to a new and different host. But the check seems to be wrong here. Also, we disallow in MainWin::newLiveDataSourceActionTriggered() having multiple mqtt live sources having the same host, if I see it correctly. Let's adrdess this outside of this patch. | |