'Re: Review Request: State machine architecture for PMC'

[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-panel-devel
Subject:    Re: Review Request: State machine architecture for PMC
From:       "Christophe Olinger" <olingerc () binarylooks ! com>
Date:       2010-04-09 7:21:15
Message-ID: 20100409072115.2204.74718 () localhost
[Download RAW message or body]

> On 2010-04-07 22:25:15, Alessandro Diaferia wrote:
> > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/mediacenterstate.h, \
> > line 73 <http://reviewboard.kde.org/r/3396/diff/7/?file=22868#file22868line73>
> > 
> > A method of a library cannot rely on a particular implementation. You are asking \
> > for a MediaLayout* to be passed in as a parameter but that class is something \
> > defined within the MediaContainment, that is within the application level. We, \
> > here, are at a library level instead.. This is indicates that probably we have do \
> > re-design this.

Yesterday I took the medialyout header from mediacenterstate.cpp and included it into \
the mediacenterstate header file. The forward declaration of the "class MediaLayout" \
I also removed from the header file. I then removed the comments in the \
mediacontainment.cpp file which now calls this and the function below from the \
mediacenterstate object but also from objects of the subclasses. For example in the \
musicstate class, the configureMediaLayout function tells the plalyist to become \
visible and the connectMediaLayout function connects a toggle Playlist button a slot \
in the medialyout. Strangely, the connection works, but as soon as I uncomment the \
call to the mediaLayout->setPlaylistVisible(true) function in the \
configureMedialayout function I get an undefined reference error for that function \
(checked spelling). ( the call to the configureMedialyout function from the \
containment to the musicstate (m_musicState->configureMedialyout(m_layout)) is not in \
svn right now, but the rest is. So there might we a reasoning error on my side as you \
said in this comment. I'll try to catch one of you guys this weekend. Marco tried to \
help me yesterday with good ideas but I had to leave on short notice :-/

- Christophe

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3396/#review4918
-----------------------------------------------------------

On 2010-04-07 20:59:11, Christophe Olinger wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3396/
> -----------------------------------------------------------
> 
> (Updated 2010-04-07 20:59:11)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> This is the first try to get state machines working. I will of course not forget \
> the previous comments. It builds and runs and state switching is possible. No \
> playbackbuttons or other useful stuff yet, but is easy to get back. 
> What I did:
> Created a mediacenterstate class which contains info about shared (so called Main) \
> subcomponents. subcomponents are little widgets like buttons and sliders which will \
> appear in the control bar. I sublcassed QState for this. Then I created a \
> picturestate and videostate class, subclasses of the follwoing which are loaded on \
> state switch. The containment does the state switch, the state classes do all the \
> rest: connections, conficurations and hading out subcomponents to the controlbar. \
> The controlbar now has a class to add widgets to itself. 
> 
> Diffs
> -----
> 
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/abstractmediaitemview.h \
> 1112197  trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/abstractmediaitemview.cpp \
> 1112197  trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/mediabrowser.h \
> 1112197  trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/mediabrowser.cpp \
> 1112197  trunk/playground/base/plasma/MediaCenterComponents/applets/mediacontroller/controller.h \
> 1112197  trunk/playground/base/plasma/MediaCenterComponents/applets/mediacontroller/controller.cpp \
> 1112197  trunk/playground/base/plasma/MediaCenterComponents/applets/mediaplayer/mediaplayer.h \
> 1112197  trunk/playground/base/plasma/MediaCenterComponents/applets/mediaplayer/mediaplayer.cpp \
> 1112197  trunk/playground/base/plasma/MediaCenterComponents/containments/mediacontainment/mediacontainment.h \
> 1112197  trunk/playground/base/plasma/MediaCenterComponents/containments/mediacontainment/mediacontainment.cpp \
> 1112197  trunk/playground/base/plasma/MediaCenterComponents/containments/mediacontainment/medialayout.h \
> 1112197  trunk/playground/base/plasma/MediaCenterComponents/containments/mediacontainment/medialayout.cpp \
> 1112197  trunk/playground/base/plasma/MediaCenterComponents/libs/CMakeLists.txt \
> 1112197  trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/CMakeLists.txt \
> 1112197  trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/browser.h \
> 1112197  trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/browser.cpp \
> 1112197  trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/mediacenter.h \
> 1112197  trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/mediacenterstate.h \
> PRE-CREATION  trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/mediacenterstate.cpp \
> PRE-CREATION  trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/musicstate.h \
> PRE-CREATION  trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/musicstate.cpp \
> PRE-CREATION  trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/picturestate.h \
> PRE-CREATION  trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/picturestate.cpp \
> PRE-CREATION  trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/playbackcontrol.h \
> 1112197  trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/playbackcontrol.cpp \
> 1112197  trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/player.h \
> 1112197  trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/videostate.h \
> PRE-CREATION  trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/videostate.cpp \
> PRE-CREATION  trunk/playground/base/plasma/MediaCenterComponents/shells/plasmediacenter/mainwindow.cpp \
> 1112197  
> Diff: http://reviewboard.kde.org/r/3396/diff
> 
> 
> Testing
> -------
> 
> State switchting works.
> 
> 
> Thanks,
> 
> Christophe
> 
> 

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

[prev in list] [next in list] [prev in thread] [next in thread] 