2 replies to this topic

[cranphin]

Hi!

I noticed the gps viewer currently gets configured twice when the app starts.
Digging some deeper I found out that first the gadget is loaded, and a 'default' configuration (index 0) is set,
and then the actual stored state is loaded.
I tried a bit to see if I can do something about that, resulting in the changes below, which seem to work.
But I don't think I like it this way, I don't know enough of the internals yet either ;)
I'm guessing instead of the state, just the configuration name could be stored and used? sounds more efficient :)
What do people think? :)

Index: src/plugins/coreplugin/uavgadgetdecorator.cpp
===================================================================
--- src/plugins/coreplugin/uavgadgetdecorator.cpp	(revision 1505)
+++ src/plugins/coreplugin/uavgadgetdecorator.cpp	(working copy)
@@ -33,7 +33,7 @@
 
 using namespace Core;
 
-UAVGadgetDecorator::UAVGadgetDecorator(IUAVGadget *gadget, QList<IUAVGadgetConfiguration*> *configurations) :
+UAVGadgetDecorator::UAVGadgetDecorator(IUAVGadget *gadget, QList<IUAVGadgetConfiguration*> *configurations, bool initialize) :
		 IUAVGadget(gadget->classId(), gadget->parent()),
		 m_gadget(gadget),
		 m_toolbar(new QComboBox),
@@ -45,8 +45,9 @@
	 foreach (IUAVGadgetConfiguration *config, *m_configurations)
		 m_toolbar->addItem(config->name());
	 connect(m_toolbar, SIGNAL(activated(int)), this, SLOT(loadConfiguration(int)));
-	if (m_configurations->count() > 0)
+	if (initialize && m_configurations->count() > 0) {
		 loadConfiguration(0);
+	}
	 updateToolbar();
 }
 
Index: src/plugins/coreplugin/uavgadgetmanager/uavgadgetview.cpp
===================================================================
--- src/plugins/coreplugin/uavgadgetmanager/uavgadgetview.cpp	(revision 1505)
+++ src/plugins/coreplugin/uavgadgetmanager/uavgadgetview.cpp	(working copy)
@@ -228,10 +228,28 @@
	 m_activeToolBar = toolBar;
 }
 
+void UAVGadgetView::bootstrap(QString classId, QByteArray state) {
+	if (indexOfClassId(classId) < 0) {
+		listSelectionActivated(m_defaultIndex);
+		return;
+	}
+
+	if (m_uavGadget && (m_uavGadget->classId() == classId)) {
+		return;
+	}
+
+	UAVGadgetInstanceManager *im = ICore::instance()->uavGadgetInstanceManager();
+	IUAVGadget *gadgetToRemove = m_uavGadget;
+	IUAVGadget *gadget = im->createGadget(classId, this, FALSE);
+	gadget->restoreState(state);
+	setGadget(gadget);
+	m_uavGadgetManager->setCurrentGadget(gadget);
+	im->removeGadget(gadgetToRemove);
+}
+
+
 void UAVGadgetView::listSelectionActivated(int index)
 {
-	if (index < 0) // this could happen when called from SplitterOrView::restoreState()
-		index = m_defaultIndex;
	 QString classId = m_uavGadgetList->itemData(index).toString();
	 if (m_uavGadget && (m_uavGadget->classId() == classId))
		 return;
@@ -612,8 +630,6 @@
		 QString classId;
		 QByteArray uavGadgetState;
		 stream >> classId >> uavGadgetState;
-		int index = m_view->indexOfClassId(classId);
-		m_view->listSelectionActivated(index);
-		gadget()->restoreState(uavGadgetState);
+		m_view->bootstrap(classId,uavGadgetState);
	 }
 }
Index: src/plugins/coreplugin/uavgadgetmanager/uavgadgetview.h
===================================================================
--- src/plugins/coreplugin/uavgadgetmanager/uavgadgetview.h	(revision 1505)
+++ src/plugins/coreplugin/uavgadgetmanager/uavgadgetview.h	(working copy)
@@ -76,6 +76,7 @@
 
 public slots:
	 void closeView();
+	void bootstrap(QString classId, QByteArray state);
	 void listSelectionActivated(int index);
 
 private slots:
Index: src/plugins/coreplugin/uavgadgetinstancemanager.h
===================================================================
--- src/plugins/coreplugin/uavgadgetinstancemanager.h	(revision 1505)
+++ src/plugins/coreplugin/uavgadgetinstancemanager.h	(working copy)
@@ -59,7 +59,7 @@
	 ~UAVGadgetInstanceManager();
	 void readConfigurations(QSettings *qs);
	 void writeConfigurations(QSettings *qs);
-	IUAVGadget *createGadget(QString classId, QWidget *parent);
+	IUAVGadget *createGadget(QString classId, QWidget *parent, bool initialize = TRUE);
	 void removeGadget(IUAVGadget *gadget);
	 bool canDeleteConfiguration(IUAVGadgetConfiguration *config);
	 void deleteConfiguration(IUAVGadgetConfiguration *config);
Index: src/plugins/coreplugin/uavgadgetdecorator.h
===================================================================
--- src/plugins/coreplugin/uavgadgetdecorator.h	(revision 1505)
+++ src/plugins/coreplugin/uavgadgetdecorator.h	(working copy)
@@ -37,7 +37,7 @@
 {
 Q_OBJECT
 public:
-	explicit UAVGadgetDecorator(IUAVGadget *gadget, QList<IUAVGadgetConfiguration*> *configurations);
+	explicit UAVGadgetDecorator(IUAVGadget *gadget, QList<IUAVGadgetConfiguration*> *configurations, bool initialize = TRUE);
	 ~UAVGadgetDecorator();
 
	 QWidget *widget() { return m_gadget->widget(); }
Index: src/plugins/coreplugin/uavgadgetinstancemanager.cpp
===================================================================
--- src/plugins/coreplugin/uavgadgetinstancemanager.cpp	(revision 1505)
+++ src/plugins/coreplugin/uavgadgetinstancemanager.cpp	(working copy)
@@ -158,13 +158,13 @@
 }
 
 
-IUAVGadget *UAVGadgetInstanceManager::createGadget(QString classId, QWidget *parent)
+IUAVGadget *UAVGadgetInstanceManager::createGadget(QString classId, QWidget *parent, bool initialize)
 {
	 IUAVGadgetFactory *f = factory(classId);
	 if (f) {
		 QList<IUAVGadgetConfiguration*> *configs = configurations(classId);
		 IUAVGadget *g = f->createGadget(parent);
-		IUAVGadget *gadget = new UAVGadgetDecorator(g, configs);
+		IUAVGadget *gadget = new UAVGadgetDecorator(g, configs, initialize);
		 m_gadgetInstances.append(gadget);
		 connect(this, SIGNAL(configurationAdded(IUAVGadgetConfiguration*)), gadget, SLOT(configurationAdded(IUAVGadgetConfiguration*)));
		 connect(this, SIGNAL(configurationChanged(IUAVGadgetConfiguration*)), gadget, SLOT(configurationChanged(IUAVGadgetConfiguration*)));

[PeterG]

loadConfiguration() gets called all the time for a gadget, when you change settings, when you choose a different configuration from the dropdown list, so personally I never saw that the benefit of not having loadConfiguration() called twice on startup  outweighed the added complexity.

Also the configuration name is exactly what is saved/restored in UAVGadgetDecorator.

The same question came up not long ago:
http://forums.openpi...g-loaded-twice/

Edited by PeterG, 03 September 2010 - 07:02 AM.

[cranphin]

PeterG, on 03 September 2010 - 05:50 AM, said:

loadConfiguration() gets called all the time for a gadget, when you change settings, when you choose a different configuration from the dropdown list, so personally I never saw that the benefits of the added complexity outweighed those of not having loadConfiguration() called twice on startup.

Also the configuration name is exactly what is saved/restored in UAVGadgetDecorator.

The same question came up not long ago:
http://forums.openpi...g-loaded-twice/

I figured it might be something like that

The reason I'm not very keen on this is that:
- A gadget might do quite some work on getting configured, for example set up a serial link to the GPS, with threads and stuff. with several gadgets in the workspace that could add up
- Inititally the 'wrong' (Default) configuration gets loaded. For some gadgets that's no problem, but I can foresee some autostarting gadgets which cause trouble over that. Again for example a GPS gadget that starts straight away with connecting to a GPS, which then might error out since there is no GPS (and the user had things set up to use telemetry or the IP stuff, not serial), (not that is does that atm., but just as a poor example ).

I'm not pushing this too hard tho, if people have good reasons for keeping things like this then that's fine with me
Gadgets do need to handle changing configuration well ofcourse (still some work to do on that for the gps thing).

And yeah, I missed the other thread XD