On Mo, 2011-08-15 at 15:05 +0200, Chris Kühl wrote:
> Also, I think I'm pretty much done Bluetooth work. I've commit my
> changes to the bluetooth-device-id branch[1] and updated the bug[2].
Would it make sense to squash the development history? I don't know how
many different commits would make sense. Right now I am only looking at
one diff that includes everything.
It adds a TODO:
+ if(hasPnpInfoService(uuids)) {
+ // TODO: Get the actual vendor and product ids.
+ DBusClientCall1<ServiceDict> discoverServices(*this,
+
"DiscoverServices");
What does that mean? Is there work left to do and is it tracked in
bugs.meego.com?
+ if(pos != std::string::npos)
+ {
Not my favorite bracket style, but so be it...
+static string SyncEvolutionDataDir()
+{
+ string dataDir(DATA_DIR);
+ const char *envvar = getenv("SYNCEVOLUTION_DATA_DIR");
+ if (envvar) {
+ dataDir = envvar;
+ }
+ return dataDir;
+}
This new env variable needs to be documented in README.rst. A better place for
this
function might be in util.h/cpp.
+static bool getPnpInfoNamesFromValues(const std::string &vendorValue,
std::string &vendorName,
+ const std::string &productValue,
std::string &productName)
+{
+ static GKeyFile *bt_key_vals = NULL;
+
+ if(!bt_key_vals) {
+ bt_key_vals = g_key_file_new();
+ GError *err = NULL;
+ string filePath(SyncEvolutionDataDir() + "/bluetooth_products.ini");
The syncevo-dbus-server has a feature where it watches files that make
up the running process and restarts when any of those change. System
daemons do that via package scripts, but for daemons started in a
session that doesn't work. The SyncEvolution mechanism currently works
for the executable and shared libraries, automatically found from
scanning /proc/self/maps.
Data files are loaded anew when needed (XML config files, for example).
Doing that for every getPnpInfoNamesFromValues() might be a bit often.
Perhaps the result can be cached in the D-Bus session instance instead
of in a static variable?
Or you could manually add that file to the watch list. See
DBusServer::run().
+ if(vendor)
+ vendorName = vendor;
+ else
+ // We at least need a vendor id match.
+ return false;
Now *that's* the (non-)bracket style which I once declared unacceptable
for SyncEvolution ;-) I know, I should replace the reference to the
Funambol coding style in the HACKING document with some actual rules.
Please, always use brackets even around single statements.
+ /** callback of 'DiscoverServices' method. The serve detals are
retrieved */
+ void discoverServicesCb(const ServiceDict &serviceDict, const
std::string &error);
Typo. Really minor. Consider it a feeble attempt at demonstrating that I
have read the patch.
for(syncDevIt = m_syncDevices.begin(); syncDevIt != m_syncDevices.end();
++syncDevIt) {
if(boost::equals(syncDevIt->m_deviceId, deviceId)) {
device = *syncDevIt;
+ if(syncDevIt->m_pnpInformation)
+ {
Switching coding style in the middle of some other code looks weird.
--- a/test/test-dbus.py
+++ b/test/test-dbus.py
@@ -53,6 +53,10 @@ monitor = ["dbus-monitor"]
# primarily for XDG files, but also other temporary files
xdg_root = "temp-test-dbus"
configName = "dbus_unittest"
+# for bluetooth tests. replace with values from your test device.
+bt_device_mac = "D4:5D:42:73:E4:6C"
+bt_device_fingerprint = "Nokia 5230"
+bt_device_name = "Nokia 5230a"
Replace how? By editing the script? I know that I have set a bad
precedent by using the same approach for enabling gdb, but that is rare.
A better mechanism for the device would be useful - command line
parameter or env variable? Or perhaps better simulate a desktop
environment with various paired Bluetooth devices by registering a mock
org.bluez implementation on the D-Bus session bus and using it in
syncevo-dbus-server when a certain env variable is set? See
DBUS_TEST_CONNMAN for an example.
There are some white space changes in test-dbus.py. I assume that that's
cleanup work which will be a separate patch.
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
_______________________________________________
SyncEvolution mailing list
[email protected]
http://lists.syncevolution.org/listinfo/syncevolution
