Details

Reviewers

Group Reviewers

Maniphest Tasks

Commits

R224:9c1d6e43ad99: Add contacts-reading plugin (KDE side)
R224:99fa6e43afc0: Add first-cut contacts-reading plugin
R224:68014ab9bd53: Upgrade TODOs to action items

Summary

Add a plugin to KDE Connect which supports exporting the Android contacts databases to vcards on the desktop

When the devices are connected, the plugin sends a request for all timestamps and IDs

When a packet with timestamps and IDs is received, it verifies it has vcards for each ID and that the timestamps match and deletes any vcards for IDs which were not reported. It then sends a request for all vcards which were missing or need updating

When a packet with vcards is received they are unconditionally written to disk, possibly overwriting existing vcards

Provides one dbus method: contacts/synchronizeRemoteWithLocal which triggers the request for all timestamps and IDs

BUG: 367999

Test Plan

Connect the device to the desktop and verify that vcards are created in QStandardPaths::GenericDataLocation / kpeoplevcard". On my system this is ~/.local/share/kpeoplevcard

Create a dummy contact on the device and verify it is synchronized (Currently not automatic, have to disconnect and reconnect or use dbus)

Modify the dummy contact and verify the modifications are synchronized (Currently not automatic, have to disconnect and reconnect or use dbus)

Delete the dummy contact and verify the deletion is synchronized (Currently not automatic, have to disconnect and reconnect or use dbus)

Diff Detail

Repository

R224 KDE Connect

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

sredman requested review of this revision.Jan 6 2018, 1:52 AM

sredman created this revision.

sredman added a reviewer: KDE Connect.Jan 6 2018, 1:55 AM

Add dbus interface for requesting unique IDs for accessing the contacts

Attach dbus to the correct path

Add interface for requesting contact names given uIDs

Change return type of getAllContactUIDs to int instead of string

Add interface for requesting phone numbers and email addresses

Add PhoneEntry container class for all the data associated with a phone number
Add interface for requesting phone numbers given a list of uIDs
Remove old interface for requesting random jumble of contact names + phone numbers
Combine sender methods into single helper
Add locks when handling a phone number response packet
Stop manually setting up DBus connection, since it is already handled
Add package type and handler for requesting contact phone numbers given a list of uIDs
Reorder methods

sredman retitled this revision from Add first-cut contacts-reading plugin to Add contacts-reading plugin.Jan 16 2018, 1:41 AM

sredman edited the summary of this revision. (Show Details)

sredman edited the test plan for this revision. (Show Details)

The DBus API seems too convoluted, I suggest using vcard for now, which will be understood by every PIM suite in existance.

apol requested changes to this revision.Mar 16 2018, 10:23 PM

This revision now requires changes to proceed.Mar 16 2018, 10:23 PM

This revision was not accepted when it landed; it landed in state Needs Revision.Mar 24 2018, 1:20 PM

Closed by commit R224:99fa6e43afc0: Add first-cut contacts-reading plugin (authored by sredman). · Explain Why

This revision was automatically updated to reflect the committed changes.

sredman reopened this revision.Mar 25 2018, 5:16 PM

Update contacts plugin for VCard API

Combine sender methods into single helper
Strip down contacts plugin for new VCards api
Merge remote-tracking branch 'origin/master' into contacts
Write VCards to local files
Move vcards storage path to class-level
Implment reading local cache for missing and remove unreported contacts
Update locally-cached vcards if the timestamp differs
Synchronize contacts when plugin loads

Cleanup TODOs

This revision was not accepted when it landed; it landed in state Needs Review.Mar 25 2018, 6:28 PM

Closed by commit R224:68014ab9bd53: Upgrade TODOs to action items (authored by sredman). · Explain Why

This revision was automatically updated to reflect the committed changes.

sredman added a task: T8283: Sync Contacts.Mar 27 2018, 4:39 PM

Restricted Application added a project: KDE Connect. · View Herald TranscriptMar 27 2018, 4:39 PM

sredman reopened this revision.Mar 27 2018, 4:41 PM

sredman retitled this revision from Add contacts-reading plugin to Add contacts-reading plugin (KDE side).Mar 27 2018, 5:21 PM

sredman edited the summary of this revision. (Show Details)

sredman edited the test plan for this revision. (Show Details)

sredman added a subscriber: KDE Connect.

Same here.

Update diff with ALL changes. Arc is too powerful for me!

Add first-cut contacts-reading plugin
Add dbus interface for requesting unique IDs for accessing the contacts
Attach dbus to the correct path
Change return type of getAllContactUIDs to int instead of string
Add interface for requesting contact names given uIDs
Add PhoneEntry container class for all the data associated with a phone number
Add interface for requesting phone numbers given a list of uIDs
Remove old interface for requesting random jumble of contact names + phone numbers
Combine sender methods into single helper
Add locks when handling a phone number response packet
Stop manually setting up DBus connection, since it is already handled
Add package type and handler for requesting contact phone numbers given a list of uIDs
Reorder methods
Change NetworkPackage to NetworkPacket
Strip down contacts plugin for new VCards api
Print VCards
Add dependency on KPeople for contacts plugin
Revert "Add dependency on KPeople for contacts plugin"
Write VCards to local files
Move vcards storage path to class-level
Implment reading local cache for missing and remove unreported contacts
Update locally-cached vcards if the timestamp differs
Synchronize contacts when plugin loads
Upgrade TODOs to action items

In D9691#235506, @mtijink wrote:

Same here.

Okay. I am now more confident that I have done this correctly :)

Some nitpicking... looking much better.
Please extrapolate the comments to similar situations.

plugins/contacts/contactsplugin.cpp
72	in KDE Connect we don't add a new line after the if: `} else if () {` Fix others too.
103	mark uIDs const
110	const QString & ID
181	auto &
plugins/contacts/contactsplugin.h
81	How about: typedef qint64 UID; typedef QList<uID_t> uIDList_t;
145	const QString &
155	const QString & and uIDs

This revision now requires changes to proceed.Mar 28 2018, 12:08 PM

mtijink added inline comments.Mar 29 2018, 7:45 PM

plugins/contacts/contactsplugin.cpp
71	It's not really common in C++ to write `this->`.
plugins/contacts/contactsplugin.h
70	You can get the uid's by calling `map.keys()`, so the separate list is not needed.
76	Wouldn't `kdeconnect/vcard` make more sense?
123	Why protected? We're not going to inherit from this, right?
plugins/contacts/kdeconnect_contacts.json
14	How will the contact integrate with the desktop? E.g. will I see the contacts in KAddressBook? Or can I sent people e-mails using this? If the contacts conflict with what the user has already set up, I'd disable it by default. Otherwise, this is fine.

Update ContactsPlugin to use LOOKUP_KEY instead of RAW_CONTACT_ID since that is what LOOKUP_KEY is for and address reviewer comments

In D9691#235891, @apol wrote:

Some nitpicking... looking much better.
Please extrapolate the comments to similar situations.

Nitpicking is good. That is how I learn!

plugins/contacts/contactsplugin.h
76	The KPeople plugin KPeopleVCard expects vcards to be in that folder
123	The future is uncertain... Maybe someone wants to make a BetterContactsPlugin which supports more packets but lives alongside this plugin? I don't see any reason to stop them.
plugins/contacts/kdeconnect_contacts.json
14	vcards are written to the folder where KPeopleVCard expects to find them. Then, once KPeople and KPeopleVCard are properly configured, any application which uses KPeople will automatically find the contacts Writing contacts back to the phone is not (yet) supported Kpeople supports merging contacts, and each KDE Connect device writes to its own folder, so if multiple contacts with the same information exist they can be merged within KPeople

sredman marked 6 inline comments as done.Mar 30 2018, 5:09 AM

sredman added inline comments.Mar 30 2018, 5:13 AM

plugins/contacts/contactsplugin.cpp
71	Does it do any harm? It's a habit from writing too much Python (which I'd rather keep so I can still writing Python) and being overly clear never hurt anybody
plugins/contacts/contactsplugin.h
70	I believe this comment meant to apply somewhere else in the code? But in general I like to give names to things so that other lines are shorter and clearer

Emit signal reporting changed vcards

sredman marked 11 inline comments as done.Apr 18 2018, 7:04 PM

Patch looks good to me, I'd say let's merge it so we can start testing.

plugins/contacts/contactsplugin.h
123	Well that's not a great reason. If we ever need more abstraction we'll have to modify the code, and that's fine. I'm not concerned about this though. Note that public before Q_SIGNAL it does nothing and can be removed.

apol accepted this revision.Apr 18 2018, 10:44 PM

This revision is now accepted and ready to land.Apr 18 2018, 10:44 PM

nicolasfella mentioned this in T8555: [RFC] Import contacts from KDE Connect.Apr 19 2018, 12:04 PM

Closed by commit R224:9c1d6e43ad99: Add contacts-reading plugin (KDE side) (authored by sredman, committed by nicolasfella). · Explain WhyApr 23 2018, 8:30 PM

This revision was automatically updated to reflect the committed changes.

		Path
M		plugins/CMakeLists.txt (1 line)
A	M	plugins/contacts/CMakeLists.txt (11 lines)
A	M	plugins/contacts/README (3 lines)
A	M	plugins/contacts/contactsplugin.h (164 lines)
A	M	plugins/contacts/contactsplugin.cpp (211 lines)
A	M	plugins/contacts/kdeconnect_contacts.json (33 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	24807	1f6544e		Jan 6 2018, 1:52 AM	★	★
Diff 2	25361	1f6544e	Add dbus interface for requesting unique IDs for accessing the contacts	Jan 15 2018, 2:27 AM	★	★
Diff 3	25363	1f6544e	Add interface for requesting contact names given uIDs	Jan 15 2018, 7:35 AM	★	★
Diff 4	25435	1f6544e	Add interface for requesting phone numbers and email addresses	Jan 16 2018, 1:15 AM	★	★
Diff 5	30389	0c560d9	R224:99fa6e43afc089d995762356f1ee26a8ad400fda	Mar 23 2018, 10:09 PM	★	★
Diff 6	30533	33c2a10	Update contacts plugin for VCard API	Mar 25 2018, 5:18 PM	★	★
Diff 7	30539	581b41b	Cleanup TODOs	Mar 25 2018, 6:28 PM	★	★
Diff 8	30540	581b41b	R224:68014ab9bd53536ae060eac8c102b36838aaadd0	Mar 25 2018, 6:27 PM	★	★
Diff 9	30769	bf327e5	Update diff with ALL changes. Arc is too powerful for me!	Mar 28 2018, 12:43 AM	★	★
Diff 10	30898	bf327e5	Update ContactsPlugin to use LOOKUP_KEY instead of RAW_CONTACT_ID since that is…	Mar 30 2018, 5:07 AM	★	★
Diff 11	31071	bf327e5	Emit signal reporting changed vcards	Mar 31 2018, 9:42 PM	★	★
Diff 12	32908	634d33b	R224:9c1d6e43ad99f51fbf0619e1ca7fca4283f122f0	Apr 23 2018, 8:30 PM	★	★

Commit	Tree	Parents	Author	Summary	Date
49499ebe6874	eb405a2807a9	bd6e2f0c9968	Simon Redman	Break vcard into variable before writing for debugging	Mar 31 2018, 8:44 PM
bd6e2f0c9968	74785b6874e6	e90a9d4e31fe	Simon Redman	Emit signal indicating cache has been updated	Mar 31 2018, 7:29 PM
e90a9d4e31fe	bd3b0b39b407	e5bc08f37d43	Simon Redman	Change to abstract uID type and use LOOKUP_KEY instead of RAW_CONTACT_ID, since… (Show More…)	Mar 30 2018, 4:56 AM
e5bc08f37d43	50f5bf72bd38	ea9ea5fa43da	Simon Redman	Where sensible, use const <type>&	Mar 30 2018, 3:32 AM
ea9ea5fa43da	94cbb64f7332	ffe68b8ee835	Simon Redman	Where sensible, use const <type>&	Mar 30 2018, 3:29 AM
ffe68b8ee835	a32804a3acb7	fc6d8bd11486	Simon Redman	Reconfigure and rerun autoformatter	Mar 30 2018, 2:55 AM
fc6d8bd11486	495bc056d7fc	592c626488a5 bf327e5b8c6c	Simon Redman	Merge remote-tracking branch 'origin/master' into contacts	Mar 27 2018, 3:11 PM
592c626488a5	8e8381b3a2ab	68014ab9bd53 bee6c4a8beab	Simon Redman	Merge remote-tracking branch 'origin/master' into contacts	Mar 25 2018, 8:23 PM
68014ab9bd53	06f15e425c85	581b41bec388	Simon Redman	Upgrade TODOs to action items	Mar 25 2018, 6:27 PM
581b41bec388	f913266054e6	2c5955c6f8af	Simon Redman	Synchronize contacts when plugin loads	Mar 25 2018, 5:11 PM
2c5955c6f8af	8299fc015ae1	eef4226b8e81	Simon Redman	Update locally-cached vcards if the timestamp differs	Mar 25 2018, 5:03 PM
eef4226b8e81	34b480445193	f2690a9ddf4a	Simon Redman	Implment reading local cache for missing and remove unreported contacts (Show More…)	Mar 25 2018, 4:41 PM
f2690a9ddf4a	495a165dba9d	9caa826bab62	Simon Redman	Move vcards storage path to class-level	Mar 25 2018, 4:39 PM
9caa826bab62	a91c89c0b339	4c82234d7c73	Simon Redman	Write VCards to local files	Mar 25 2018, 1:59 PM
4c82234d7c73	a7a7a69219c8	b7cbfaa2b509	Simon Redman	Revert "Add dependency on KPeople for contacts plugin" (Show More…)	Mar 25 2018, 12:37 PM
b7cbfaa2b509	851401e3b31a	4261123f7874 33c2a100b1cc	Simon Redman	Merge remote-tracking branch 'origin/master' into contacts	Mar 25 2018, 12:18 PM
4261123f7874	085cefcfdfca	e54de00c4139	Simon Redman	Add dependency on KPeople for contacts plugin	Mar 25 2018, 12:18 PM
e54de00c4139	43144c609439	2a7b05cd3146	Simon Redman	Print VCards	Mar 25 2018, 11:59 AM
2a7b05cd3146	2aa5ec27c3d8	a19ffdb7e22d	Simon Redman	Strip down contacts plugin for new VCards api	Mar 25 2018, 9:32 AM
a19ffdb7e22d	6ee9e07e69f4	6e5c07571c81	Simon Redman	Change NetworkPackage to NetworkPacket	Mar 24 2018, 1:19 PM
6e5c07571c81	25c1b3da7f0f	1a90e73ccfbc	Simon Redman	Reorder methods	Jan 16 2018, 1:09 AM
1a90e73ccfbc	612a01bcd9e3	6744d10d2b2b	Simon Redman	Add package type and handler for requesting contact phone numbers given a list… (Show More…)	Jan 16 2018, 1:05 AM
6744d10d2b2b	08c336d4b1b6	66d174daa636	Simon Redman	Stop manually setting up DBus connection, since it is already handled	Jan 16 2018, 1:03 AM
66d174daa636	aa3bc8294828	c33af1ae4730	Simon Redman	Add locks when handling a phone number response packet	Jan 16 2018, 12:06 AM
c33af1ae4730	e2b22c050325	5d3b0f53c796	Simon Redman	Combine sender methods into single helper	Jan 16 2018, 12:03 AM
5d3b0f53c796	aa204ac321d7	804ad1aaecef	Simon Redman	Remove old interface for requesting random jumble of contact names + phone… (Show More…)	Jan 15 2018, 11:01 PM
804ad1aaecef	7a0da006f284	a07d8b8e1d45	Simon Redman	Add interface for requesting phone numbers given a list of uIDs	Jan 15 2018, 10:51 PM
a07d8b8e1d45	3ea3ab0acfdf	738d4147a1c5	Simon Redman	Add PhoneEntry container class for all the data associated with a phone number	Jan 15 2018, 10:46 PM
738d4147a1c5	39f1b8544076	62a9ccadcd85	Simon Redman	Add interface for requesting contact names given uIDs	Jan 15 2018, 7:35 AM
62a9ccadcd85	2cbb3ef86906	82a962444d8c	Simon Redman	Change return type of getAllContactUIDs to int instead of string	Jan 15 2018, 4:41 AM
82a962444d8c	27ecb031a9fa	30774f63faa7	Simon Redman	Attach dbus to the correct path	Jan 15 2018, 2:26 AM
30774f63faa7	ba77316496b8	99fa6e43afc0	Simon Redman	Add dbus interface for requesting unique IDs for accessing the contacts	Jan 15 2018, 2:26 AM
99fa6e43afc0	9a8663cce8f4	0c560d9062bd	Simon Redman	Add first-cut contacts-reading plugin (Show More…)	Jan 6 2018, 1:51 AM

Add contacts-reading plugin (KDE side)
ClosedPublic
Actions

Details

Diff Detail

Revision Contents
Changeset List

Diff 32908

plugins/CMakeLists.txt

plugins/contacts/CMakeLists.txt

plugins/contacts/README

plugins/contacts/contactsplugin.h

plugins/contacts/contactsplugin.cpp

plugins/contacts/kdeconnect_contacts.json

Add contacts-reading plugin (KDE side)ClosedPublicActions

Details

Diff Detail

Revision ContentsChangeset List

Diff 32908

plugins/CMakeLists.txt

plugins/contacts/CMakeLists.txt

plugins/contacts/README

plugins/contacts/contactsplugin.h

plugins/contacts/contactsplugin.cpp

plugins/contacts/kdeconnect_contacts.json

Add contacts-reading plugin (KDE side)
ClosedPublic
Actions

Revision Contents
Changeset List