Aurélien Gâteau wrote:
Vadim Lebedev a écrit :
It is much better to have couple of phrases of comments in free-flow
format
in 2-3 lines, than a 'beautiful' comment templates on half a page,
which often make impossible to see
the comment and function body together on the monitor. ( Of course
there is another reason to use comment templates and it's name
'Doxygen' so i'll have to live with them :(.... )
Aurelien,
thank you for sharing your thoughts.
First, i've forgotten to attach my latest patch... so'im attaching it to
this message
Hello Vadim,
Just wanted to point out two things about Doxygen comments:
First: if I'm not mistaken, OpenWengo project coding style says that
Doxygen comments should be in .h files, so they should not annoy you
when reading source code.
Second: if you haven't tried it, I suggest you have a look at Doxygen
source code output. It's a nice way to explore existing code (at least
it was really useful for me to get started on the OpenWengo project,
and it still is when I am wandering into parts of the code I don't
know yet).
The nice thing about this output is that:
- Big Doxygen comment templates have been removed.
- All classes and methods are hyperlinked, making it really convenient
to browse from one method call to its doc and then to its implementation.
Now,
I'm perfectly aware of Doxygen capabilities and i do agree that Doxygen
compatibility is a GOOD thing
for coding style rules.
However most of the time coders spend in the editor window and not
looking to doxygen output and inside
the editor all this doxygen stuff is pretty annoying. As i said before
the positive effect of doxygen compatiblity
IMO outweighs the negiative effect.
Here is a (completely random example): the implementation of
WebcamDriver::setPalette():
http://dev.openwengo.org/doxygen/openwengo/wengophone-ng/branches/wengophone-2.1/HEAD/html/_webcam_driver_8cpp-source.html#l00188
So with the time i've devised what i believe the optimal approach to
coding style rules:
1) Publicly visible files (module interface *.h for example) follow
whatever coding style rules are established for the project.
2) Internal files follows the coding style of the original author.
3) Project coding style rules are not carved in the stone, people may
propose modifications which are accepted/rejected by majority vote.
The outcome of this approach, is that it does not hampers personal
creativity of the coders and leaves them
experimentation space, so they could learn and try new things.
So how about to try my approach for openwengo?
I see your point, but I think this method has a major drawback: when
someone needs to implement some kind of transverse change, he may have
to adjust to a different coding style for each file he edit.
On this point however we've a disagreement.
I believe that what you see as amajor drawback is MAJOR ADVANTAGE instead.
This way a person have to try something differrent that he is doing usually
which gives him a chance to learn something new and become better coder.
The important thing that it comes
in small doses so the person does not feel himeself defintively
'forced'. Everebody is capable to adopt
some different 'accent' for a short moment.
This is also true for maintainers having to go through several files.
With different coding styles the learning process is less dull, which
improves overall efficiency.
Don't forget that coders (especially good ones) are not 'machines'.
They are creative personalities, they don't
like dull tasks. It's the managers who usually do like formulars and
stricts rules. But in reality, my personal experience shows that this
simply gives them an 'illusion' of control and too strict rules are
detrimental to overall project success. This is actually a sign of COA
(Cover-your-ass) approach.
In the end, I believe a properly enforced coding style is always
painful on some particular point, but is always better than having no
coding style.
I don't advocate abandon of coding style rules, i simply say that they
should be really enforced only in *.h files.
I'd like to hear the opinion of other coders on the list.
BTW i'd like to stress, that i'm not talking in the air here, my 3
rules are outcome of some 30 years of coding and
project management experience and actually were applied with great
success in large projects.
Vadim
Index: imwrapper/include/imwrapper/IMWrapperFactory.h
===================================================================
--- imwrapper/include/imwrapper/IMWrapperFactory.h (revision 10772)
+++ imwrapper/include/imwrapper/IMWrapperFactory.h (working copy)
@@ -28,7 +28,8 @@
class IMConnect;
class IMPresence;
class IMContactList;
-
+class IMPager;
+
/**
* Factories for the Instant Messaging wrapper component.
*
@@ -103,7 +104,16 @@
* @return the new IMContactList instance.
*/
virtual IMContactList * createIMContactList(IMAccount & account) = 0;
+
+
+ /**
+ * Instantiates a IMPager implementation.
+ *
+ * @return the new IMPager instance.
+ */
+ virtual IMPager * createIMPager(const std::string &smsServer) = 0;
+
/**
* Tells the wrapper to remove all class associated with the given IMAccount.
* TODO: this is a workaround until the refactoring from CoIpManager branch.
Index: imwrapper/include/imwrapper/IMPager.h
===================================================================
--- imwrapper/include/imwrapper/IMPager.h (revision 0)
+++ imwrapper/include/imwrapper/IMPager.h (revision 0)
@@ -0,0 +1,97 @@
+/*
+ * WengoPhone, a voice over Internet phone
+ * Copyright (C) 2004-2006 Wengo
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#ifndef OWIMPAGER_H
+#define OWIMPAGER_H
+
+
+#include <util/Event.h>
+#include <util/Interface.h>
+#include <util/Trackable.h>
+
+#include <string>
+#include <list>
+
+
+/**
+ * Wrapper for sending Pager mode Instant Messages
+ *
+ * @author Vadim Lebedev
+ */
+class IMPager : Interface, public Trackable {
+public:
+
+ /**
+ * Emitted when a new Page Mode message arrives.
+ *
+ * @param from SIP originator
+ * @param ctt content type
+ * @param message message content
+ */
+ Event<void (const std::string & from, const std::string & ctt, const std::string & message )> sipMessageEvent;
+
+ /**
+ * Emitted when a new SMS Mode message arrives.
+ *
+ * @param ctt content type
+ * @param message message content
+ */
+ Event<void (const std::string & ctt, const std::string & message )> smsMessageEvent;
+
+ /**
+ * Emitted when a new SMS Mode message arrives.
+ *
+ * @param ctt content type
+ * @param message message content
+ */
+ Event<void (const std::string & ctt, const std::string & message )> smsDeliveredEvent;
+
+
+ /**
+ * Send SIP MESSAGE request
+ *
+ * @param to destination URI
+ * @param ctt content type
+ * @param message message text
+ */
+ virtual void sendSipMessage(const std::string &to, const std::string &ctt, const std::string & message) = 0;
+
+
+ /**
+ * Send SMS MESSAGE using SIP-SMS server
+ *
+ * @param tos list of SMS destinations URIs
+ * @param subj message subject
+ * @param ctt content type
+ * @param message message text
+ * @param msgid generated message id (can be used to match delivery notifications)
+ */
+ virtual void sendSmsMessage(const std::list<std::string> & tos, const std::string &subj,
+ const std::string &ctt, const std::string & message, std::string &msgid) = 0;
+
+
+
+ IMPager(const std::string &smsServerUri) : smsServer(smsServerUri) { }
+
+protected:
+ std::string smsServer;
+};
+
+
+#endif //OWIMPAGER
Index: imwrapper/src/multiim/multiim/MultiIMFactory.h
===================================================================
--- imwrapper/src/multiim/multiim/MultiIMFactory.h (revision 10772)
+++ imwrapper/src/multiim/multiim/MultiIMFactory.h (working copy)
@@ -47,6 +47,8 @@
virtual IMContactList * createIMContactList(IMAccount & account);
virtual void removeIMAccount(IMAccount imAccount);
+
+ virtual IMPager * createIMPager(const std::string &smsServer);
private:
Index: imwrapper/src/multiim/MultiIMFactory.cpp
===================================================================
--- imwrapper/src/multiim/MultiIMFactory.cpp (revision 10772)
+++ imwrapper/src/multiim/MultiIMFactory.cpp (working copy)
@@ -56,11 +56,11 @@
} else {
return _gaimIMFactory.createIMChat(account);
}
-}
-
+}
+
IMPresence * MultiIMFactory::createIMPresence(IMAccount & account) {
if ((account.getProtocol() == EnumIMProtocol::IMProtocolSIPSIMPLE)
- || (account.getProtocol() == EnumIMProtocol::IMProtocolSIP)
+ || (account.getProtocol() == EnumIMProtocol::IMProtocolSIP)
|| (account.getProtocol() == EnumIMProtocol::IMProtocolWengo)) {
return _phApiFactory.createIMPresence(account);
} else {
@@ -68,6 +68,11 @@
}
}
+
+IMPager * MultiIMFactory::createIMPager(const std::string &smsServer) {
+ return _phApiFactory.createIMPager(smsServer);
+}
+
IMContactList * MultiIMFactory::createIMContactList(IMAccount & account) {
if ((account.getProtocol() == EnumIMProtocol::IMProtocolSIPSIMPLE)
|| (account.getProtocol() == EnumIMProtocol::IMProtocolSIP)
@@ -76,8 +81,10 @@
} else {
return _gaimIMFactory.createIMContactList(account);
}
-}
+}
+
+
void MultiIMFactory::removeIMAccount(IMAccount imAccount) {
if ((imAccount.getProtocol() == EnumIMProtocol::IMProtocolSIPSIMPLE)
|| (imAccount.getProtocol() == EnumIMProtocol::IMProtocolSIP)
Index: imwrapper/src/gaim/GaimIMFactory.h
===================================================================
--- imwrapper/src/gaim/GaimIMFactory.h (revision 10772)
+++ imwrapper/src/gaim/GaimIMFactory.h (working copy)
@@ -58,7 +58,9 @@
virtual IMPresence *createIMPresence(IMAccount & account);
virtual IMContactList *createIMContactList(IMAccount & account);
-
+
+ virtual IMPager * createIMPager(const std::string &smsServer) { return 0; }
+
virtual void removeIMAccount(IMAccount imAccount);
static bool equals(const IMAccount & imAccount,
Index: sipwrapper/src/phapi/PhApiFactory.h
===================================================================
--- sipwrapper/src/phapi/PhApiFactory.h (revision 10772)
+++ sipwrapper/src/phapi/PhApiFactory.h (working copy)
@@ -50,6 +50,8 @@
virtual IMChat * createIMChat(IMAccount & account);
virtual IMPresence * createIMPresence(IMAccount & account);
+
+ virtual IMPager * createIMPager(const std::string &smsServer);
virtual IMContactList * createIMContactList(IMAccount & account);
Index: sipwrapper/src/phapi/PhApiCallbacks.cpp
===================================================================
--- sipwrapper/src/phapi/PhApiCallbacks.cpp (revision 10772)
+++ sipwrapper/src/phapi/PhApiCallbacks.cpp (working copy)
@@ -544,7 +544,16 @@
if (info->szSubContentType) {
subtype = info->szSubContentType;
}
-
+
+ std::string ctt = ctype;
+ ctt += "/";
+ ctt += subtype;
+
+ if (info->event == MESSAGE_NEW) {
+ std::string to = p->parseFromHeader(std::string(info->szLocalIdentity));
+ p->sipMessageEvent(*p, to, from, ctt, content);
+ }
+
//Getting buddy icon
if ((info->event == MESSAGE_NEW) && (ctype == "buddyicon")) {
if (!subtype.empty()) {
@@ -555,7 +564,7 @@
// Is there already an IMChatSession with this contact?
IMChatSession *imChatSession = p->getIMChatSession(from);
-
+
//Drop typingstate packet if there is no chat session created
if (!imChatSession && (ctype == "typingstate" || (ctype == "application" && subtype == "im-iscomposing+xml"))) {
return;
@@ -605,7 +614,7 @@
} else {
// once a message is received, typing is inferred off
p->typingStateChangedEvent(*p, *imChatSession, from, IMChat::TypingStateNotTyping);
- p->messageReceivedEvent(*p, *imChatSession, from, content);
+ p->messageReceivedEvent(*p, *imChatSession, from, content);
}
break;
Index: sipwrapper/src/phapi/PhApiWrapper.cpp
===================================================================
--- sipwrapper/src/phapi/PhApiWrapper.cpp (revision 10772)
+++ sipwrapper/src/phapi/PhApiWrapper.cpp (working copy)
@@ -927,3 +927,12 @@
return result;
}
+
+
+
+void PhApiWrapper::sendMessage(const std::string & to, const std::string & ctt, const std::string & message)
+{
+ int mid;
+
+ owplMessageSend(_wengoVline, to.c_str(), ctt.c_str(), message.c_str(), &mid);
+}
Index: sipwrapper/src/phapi/PhApiIMPager.cpp
===================================================================
--- sipwrapper/src/phapi/PhApiIMPager.cpp (revision 0)
+++ sipwrapper/src/phapi/PhApiIMPager.cpp (revision 0)
@@ -0,0 +1,206 @@
+/*
+ * WengoPhone, a voice over Internet phone
+ * Copyright (C) 2007 MBDSYS
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+
+#include <algorithm>
+#include <boost/function.hpp>
+#include <boost/bind.hpp>
+#include <ctype.h>
+#include <time.h>
+#include <sstream>
+#include <boost/algorithm/string.hpp>
+#include <boost/tokenizer.hpp>
+#include <boost/format.hpp>
+
+#include "PhApiIMPager.h"
+#include "PhApiWrapper.h"
+
+
+PhApiIMPager::PhApiIMPager(const std::string & smsServer, PhApiWrapper & phApiWrapper)
+ : IMPager(smsServer),
+ _phApiWrapper(phApiWrapper) {
+
+
+ _phApiWrapper.sipMessageEvent +=
+ boost::bind(&PhApiIMPager::msgReceivedEventHandler, this, _1, _2, _3, _4, _5);
+
+
+ myself = phApiWrapper._wengoSipAddress;
+}
+
+
+
+namespace myprivate {
+using namespace boost;
+using namespace std;
+
+
+/*
+ * CR/LF detector to use with boost::tokenizer
+ */
+template<class T> struct crlf_det;
+template<>
+struct crlf_det<char> {
+
+ template<typename Iter, typename Token>
+ inline bool operator()(Iter& next, Iter end, Token& tok)
+ {
+ const char *beos = &(*next);
+ const char *cr = (const char *)memchr(beos, '\r', end - next);
+ if (!cr || cr[1] != '\n')
+ return false;
+
+ Iter crptr = next;
+ crptr += cr - beos; // make crptr point to found CR
+
+ tok = string(next, crptr); // extract the token
+ next = crptr + 2; // advance past the LF
+ return true;
+ }
+ inline void reset() { }
+};
+
+
+static string getHeaderValue(const char *hdr, const string & data, bool tolower=false)
+{
+ static string nullstr;
+ string hstr = hdr;
+ tokenizer<crlf_det<char> > lines(data);
+
+
+ for(tokenizer<crlf_det<char> >::iterator it = lines.begin(); it != lines.end(); it++)
+ {
+ if (istarts_with(*it, hdr))
+ {
+ string res = (*it).substr(hstr.length());
+
+ if (tolower)
+ to_lower(res);
+ trim(res);
+ return res;
+ }
+
+ }
+
+ return nullstr;
+}
+
+
+static std::string getPayloadType(const std::string& data)
+{
+ return getHeaderValue("Content-Type:", data, true);
+}
+
+}
+
+void PhApiIMPager::msgReceivedEventHandler(PhApiWrapper & sender, const std::string & to,
+ const std::string & from, const std::string & ctt, const std::string & message) {
+
+ if (message.empty())
+ return;
+
+ std::string lctt = boost::to_lower_copy(ctt);
+ if (lctt != "message/cpim") {
+ sipMessageEvent(from, ctt, message);
+ return;
+ }
+
+ std::string payloadType = myprivate::getPayloadType(message);
+
+ if (payloadType == "text/html")
+ smsMessageEvent(payloadType, message);
+ else if (payloadType == "message/imdn+xml")
+ smsDeliveredEvent(payloadType, message);
+ else {
+ // FIXME: LOG something
+ }
+
+
+}
+
+
+
+void PhApiIMPager::sendSipMessage(const std::string &to, const std::string &ctt, const std::string & message) {
+
+ if (!message.empty()) {
+ _phApiWrapper.sendMessage(to, ctt, message);
+ }
+}
+
+
+static std::string maketmstamp()
+{
+ struct tm tmbuf;
+ time_t now;
+ char tmstr[64];
+
+
+ now = time(0);
+ tmbuf = *localtime(&now);
+
+ strftime(tmstr, 64, "%d-%b-%Y %I:%M:%S %P", &tmbuf);
+
+ return tmstr;
+
+}
+
+
+void PhApiIMPager::sendSmsMessage(const std::list<std::string> & tos, const std::string &subj,
+ const std::string &ctt, const std::string & message, std::string &msgid) {
+
+
+ if (!message.empty())
+ return;
+
+ msgid = "this is msgid";
+
+ std::string msg;
+ class builder {
+ public:
+ std::string ⌖
+
+ void add(const char *hdr, const std::string &v) { target += hdr; target += v; target += "\r\n"; }
+ void add(const char *hdr, const char* v) { target += hdr; target += v; target += "\r\n"; }
+ builder(std::string &t) : target(t) {}
+ };
+ builder bh(msg);
+ // std::ostringstream os;
+
+
+
+ bh.add("From: ", myself);
+ for( std::list<std::string>::const_iterator it = tos.begin(); it != tos.end(); it++)
+ bh.add("To: ", *it);
+
+ bh.add("DateTime: ", maketmstamp());
+ bh.add("Subject: ", subj);
+ bh.add("Expires: ", "3600");
+ bh.add("imdn.Message-ID: ", msgid);
+ bh.add("imdn.Disposition-Notification: ", "");
+ bh.add("Content-Type: ", ctt);
+ // os << message.length();
+ bh.add("Content-Length: ", (boost::format("%1%") % message.length()).str());
+ msg += message;
+
+ sendSipMessage(this->smsServer, "message/cpim", msg);
+
+
+}
+
+
Index: sipwrapper/src/phapi/PhApiWrapper.h
===================================================================
--- sipwrapper/src/phapi/PhApiWrapper.h (revision 10772)
+++ sipwrapper/src/phapi/PhApiWrapper.h (working copy)
@@ -53,7 +53,9 @@
* @author Mathieu Stute
* @author Julien Bossart
*/
-class PhApiWrapper : public SipWrapper {
+class PhApiWrapper : public SipWrapper {
+ friend class PhApiIMChat;
+ friend class PhApiIMPager;
public:
Event<void (PhApiWrapper & sender, IMChatSession & imChatSession)> newIMChatSessionCreatedEvent;
@@ -82,6 +84,15 @@
Event<void (PhApiWrapper & sender, const std::string & contactId, const std::string filename)> contactIconChangedEvent;
+ Event<void (PhApiWrapper & sender, const std::string & to,
+ const std::string & from, const std::string & ctt, const std::string & message)> sipMessageEvent;
+
+ Event<void (PhApiWrapper & sender, const std::string & to,
+ const std::string & from, const std::string & ctt, const std::string & body)> sipNotifyEvent;
+
+
+
+public:
/** Value when phApi returns an error message. */
static const int PhApiResultNoError = 0;
@@ -213,6 +224,9 @@
void removeContact(IMChatSession & chatSession, const std::string & contactId);
/** @} */
+
+
+ void sendMessage(const std::string & to, const std::string & ctt, const std::string & message);
/**
* @name IMPresence Implementation
Index: sipwrapper/src/phapi/PhApiIMPager.h
===================================================================
--- sipwrapper/src/phapi/PhApiIMPager.h (revision 0)
+++ sipwrapper/src/phapi/PhApiIMPager.h (revision 0)
@@ -0,0 +1,76 @@
+/*
+ * WengoPhone, a voice over Internet phone
+ * Copyright (C) 2007 MBDSYS
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#ifndef PHAPIIMPAGER_H
+#define PHAPIIMPAGER_H
+
+
+#include <imwrapper/IMPager.h>
+
+/**
+ * PhApi IM Pager.
+ *
+ * @ingroup model
+ * @author Vadim Lebedev
+ */
+class PhApiIMPager : public IMPager {
+ friend class PhApiFactory;
+ friend class PhApiWrapper;
+ friend class PhApiCallbacks;
+public:
+
+ /**
+ * Send SIP MESSAGE request
+ *
+ * @param to destination URI
+ * @param ctt content type
+ * @param message message text
+ */
+ virtual void sendSipMessage(const std::string &to, const std::string &ctt, const std::string & message);
+
+
+ /**
+ * Send SMS MESSAGE using SIP-SMS server
+ *
+ * @param tos list of SMS destinations URIs
+ * @param subj message subject
+ * @param ctt content type
+ * @param message message text
+ * @param msgid generated message id (can be used to match delivery notifications)
+ */
+ virtual void sendSmsMessage(const std::list<std::string> & tos, const std::string &subj,
+ const std::string &ctt, const std::string & message, std::string &msgid);
+
+
+
+protected:
+
+ PhApiIMPager(const std::string & smsServer, PhApiWrapper & phApiWrapper);
+
+
+ void msgReceivedEventHandler(PhApiWrapper & sender, const std::string & to,
+ const std::string & from, const std::string & ctt, const std::string & message);
+
+
+
+ PhApiWrapper & _phApiWrapper;
+ std::string myself;
+};
+
+#endif //PHAPIIMPAGER_H
Index: sipwrapper/src/phapi/PhApiFactory.cpp
===================================================================
--- sipwrapper/src/phapi/PhApiFactory.cpp (revision 10772)
+++ sipwrapper/src/phapi/PhApiFactory.cpp (working copy)
@@ -22,7 +22,8 @@
#include "PhApiWrapper.h"
#include "PhApiIMConnect.h"
#include "PhApiIMChat.h"
-#include "PhApiIMPresence.h"
+#include "PhApiIMPresence.h"
+#include "PhApiIMPager.h"
PhApiFactory::PhApiFactory() {
_phApiWrapperInstance = PhApiWrapper::getInstance();
@@ -48,8 +49,14 @@
IMPresence * PhApiFactory::createIMPresence(IMAccount & account) {
return new PhApiIMPresence(account, *_phApiWrapperInstance);
-}
+}
+
+
+IMPager * PhApiFactory::createIMPager(const std::string &smsServer) {
+ return new PhApiIMPager(smsServer, *_phApiWrapperInstance);
+}
+
IMContactList * PhApiFactory::createIMContactList(IMAccount & account) {
return NULL;
}
Index: sipwrapper/src/phapi/CMakeLists.txt
===================================================================
--- sipwrapper/src/phapi/CMakeLists.txt (revision 10772)
+++ sipwrapper/src/phapi/CMakeLists.txt (working copy)
@@ -34,6 +34,7 @@
PhApiSFPCallbacks.cpp
PhApiSFPWrapper.cpp
PhApiSFPEvent.cpp
+ PhApiIMPager.cpp
)
if (PHAPI_VIDEO_SUPPORT)
_______________________________________________
Wengophone-devel mailing list
[email protected]
http://dev.openwengo.com/mailman/listinfo/wengophone-devel
