http://bugzilla.moblin.org/show_bug.cgi?id=6378
--- Comment #23 from yongsheng zhu <yongsheng.zhu(a)intel.com> 2010-02-25 17:07:37
PST ---
(In reply to comment #22)
> A task in the list is checked when the interval has passed and
then
> it has to be checked whether there is an existing same task in
> the scheduling queue of auto sync manager. Once actived, it is put in.
Without looking at the code I don't understand this. What is the task list?
What is the scheduling queue? How are entries in both related? It shouldn't
be necessary to read the code to understand the commit message.
I'll refine
it.
+ /**
+ * A single task for automatic sync.
+ * Each task contains one peer name, peer duration and peer url.
+ * Once it is loaded by AutoSyncManager, auto sync manager will do sync
+ * according to these informations.
+ */
+ class AutoSyncTask
+ {
+ public:
+ string m_peer;
+ unsigned int m_peerDuration;
+ string m_url;
I can see that a task has these members, that doesn't have to be
documented ;-) But what do they do resp. what do they contain? I can
guess the answers, but others might not know, and I shouldn't have to
guess.
ACK.
+ /** map to contain all peers that enable automatic sync */
+ typedef std::map<unsigned int, boost::shared_ptr<AutoSyncTaskList> >
PeerMap;
+ PeerMap m_peerMap;
Maps what to an task list?
a map to store peer information related to configs'
auto sync
+ /**
+ * It reads all peers which are enabled to do auto sync and store them in
+ * the m_peerMap and then add timeout sources in the main loop to schedule
+ * auto sync tasks.
+ */
+ void init();
When a config is modified, is this reinitialized? I think a
brute-force approach is sufficient: just wipe out all pending tasks
and recreate them based on the config. This can (should?) be done at
the end of a Session which made config changes, in case that the owner
of that session makes multiple config changes.
+/**
* Implements the main org.syncevolution.Server interface.
*
* All objects created by it get a reference to the creating
@@ -693,11 +910,17 @@ class DBusServer : public DBusObjectHelper
DBusConnectionPtr m_connmanConn;
friend class Session;
+ friend class AutoSyncManager;
Why is it necessary to let AutoSyncManager break the encapsulation of
DBusServer?
That's a dilemma. I want to use PresentStatus, but it is a private
class in
DBusServer. If I make it as public, then I have to put definition of this class
it first before defining its a variable. so I will re-order the layout of
DBusServer or move the class out. I don't want bigger change. If you object it,
I'll change it.
class DBusServer {
private:
class PresenceStatus{
} m_presence;
public:
...
};
+ /** two timers to record when the http and bt are present */
+ Timer m_httpTimer;
+ Timer m_btTimer;
Not when - how long, right?
- :m_httpPresence (false), m_btPresence (false), m_initiated
(false), m_server (server)
+ :m_httpPresence (false), m_btPresence (false), m_initiated
(false), m_server (server),
+ m_httpTimer(15*60*100), m_btTimer(15*60*100)
Is this supposed to be 15 minutes? Then it has to be 15*60*1000 (Timer
takes milliseconds). This constant should be configurable, for tuning purposes.
+ if(httpChanged) {
+ m_httpTimer.reset();
+ }
+ if(btChanged) {
+ m_btTimer.reset();
+ }
Again, what do these timers count? Shouldn't they count time since the
transports become available? This looks like they count time since the
last change.
Yes, it records the latest change of status. If http is present, then
it mean
the time of transport available. If not, it means the time of transport
unavilable, make sense?
+bool AutoSyncManager::taskLikelyToRun(const AutoSyncTask
&syncTask)
+{
+ DBusServer::PresenceStatus &status = m_server.getPresenceStatus();
+ // avoid doing any checking of task list if http and bt presence are false
+ if(!status.getHttpPresence() && !status.getBtPresence()) {
+ return false;
+ }
+
+ if(boost::istarts_with(syncTask.m_url, "http") &&
status.getHttpPresence()) {
+ // don't add duplicate tasks
+ if(!findTask(syncTask)) {
+ Timer& timer = status.getHttpTimer();
+ // if the time peer have been around is longer than
'autoSyncPeerDuration',
+ // then add it into task list
+ if(timer.timeout(syncTask.m_peerDuration * 60 * 100)) {
+ return true;
+ }
+ } else if (boost::istarts_with(syncTask.m_url, "obex-bt") &&
status.getBtPresence()) {
+ // don't add duplicate tasks
+ if(!findTask(syncTask)) {
+ return true;
+ }
+ }
+ }
+ return false;
+}
This looks wrong. The check for obex-bt is inside the HTTP check?
oh, yes,
that's a merge problem. sorry.
--
Configure bugmail:
http://bugzilla.moblin.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching someone on the CC list of the bug.