On Thu, Aug 18, 2011 at 4:03 PM, Patrick Ohly <patrick.ohly(a)intel.com> wrote:
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.
Absolutely. I've squashed it into 2 commits; one for the server code
and another for the tests. You can find it in the
bluetooth-device-id-merge-prep branch.
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?
Oop. That's old. Removed now
+ if(pos != std::string::npos)
+ {
Not my favorite bracket style, but so be it...
Think I've changed them all to open on the same line as the if
statement now. [Grumbles about function and if statement braces being
different. ;)]
+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.
Ok I've updated README.rst and moved it to utils.
This is just a copy of SyncEvolutionTemplateDir which is static in
SyncConfig.cpp. I'll make a separate commit to do the same with that,
too.
+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().
Ah, yes. I did notice that code but had forgotten about it.
+ 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.
Sure, I should be done now.
+ /** 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.
Thanks. Fixed.
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.
What can I say? I like variety. ;) Fixed.
--- 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.
Ok, I see. I'll get this changed. Not in right now, though.
There are some white space changes in test-dbus.py. I assume that
that's
cleanup work which will be a separate patch.
Oops. I've removed those changes. These are in my test-dbus branch anyway.
Cheers,
Chris