Bluetooth Menu

Merge lp://staging/~charlesk/indicator-bluetooth/gmenuify into lp://staging/indicator-bluetooth/13.10

  1. gmenuify
  2. Merge into trunk
Proposed by Charles Kerr
Status: Merged
Approved by: Ted Gould
Approved revision: 88
Merged at revision: 53
Proposed branch: lp://staging/~charlesk/indicator-bluetooth/gmenuify
Merge into: lp://staging/indicator-bluetooth/13.10
Diff against target: 2990 lines (+1902/-900)
25 files modified
Makefile.am (+6/-1)
NEWS (+6/-0)
configure.ac (+36/-49)
data/Makefile.am (+7/-1)
data/com.canonical.indicator.bluetooth (+10/-0)
debian/control (+3/-6)
po/POTFILES.in (+13/-2)
po/indicator-bluetooth.pot (+0/-72)
src/Makefile.am (+21/-34)
src/bluetooth.vala (+80/-0)
src/bluez.vala (+397/-0)
src/config.vapi (+0/-3)
src/desktop.vala (+286/-0)
src/device.vala (+156/-0)
src/indicator-bluetooth.vala (+0/-142)
src/indicator3-0.4.vapi (+0/-145)
src/killswitch.vala (+167/-0)
src/libido3-0.1.vapi (+0/-10)
src/main.vala (+34/-0)
src/org-bluez.vala (+293/-0)
src/phone.vala (+76/-0)
src/profile.vala (+159/-0)
src/service.vala (+100/-435)
vapi/config.vapi (+8/-0)
vapi/rfkill.vapi (+44/-0)
To merge this branch: bzr merge lp://staging/~charlesk/indicator-bluetooth/gmenuify
Reviewer Review Type Date Requested Status
Ted Gould (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Mathieu Trudel-Lapierre Approve
Charles Kerr (community) Needs Fixing
Review via email: mp+178756@code.staging.launchpad.net

Commit message

Add phone profile. Export menus & actions using gio. Drops the gtk, dbusmenu, and libindicator build dependencies. Drops runtime dependency on gnome-blueooth in the phone profile.

Description of the change

GMenu version of indicator-bluetooth.

Because this also drops the runtime requirement for gnome-bluetooth on the phone, this another Big Merge proposal. When you swap out the front end and swap out the back end... there's not much left to keep :P

Bluetooth
=========
* KillSwitch: simple interface class for monitoring software / hardware killswitches to disable bluetooth
* Device: plain old data structure giving information about a bluetooth device
* Bluetooth: interface class for Bluetooth backend, gives connection information & Device list
* org.bluez: DBus proxy declarations
* Bluez: Bluetooth implementation which uses Bluez over DBus and a killswitch.
* RfKillSwitch: Killswitch implementation for Linux using /dev/rfkill

GUI Classes
===========
* profile: base class for profile-specific actions & menus
* phone: subclass of profile
* desktop: subclass of profile

Service
=======
* service: a simple boilerplate that owns the busname, instantiates the profiles, exports the menus & actions
* main: really ties the room together, man.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:73
http://jenkins.qa.ubuntu.com/job/indicator-bluetooth-ci/2/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/indicator-bluetooth-saucy-amd64-ci/1/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/indicator-bluetooth-saucy-armhf-ci/1/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/indicator-bluetooth-ci/2/rebuild

review: Needs Fixing (continuous-integration)
lp://staging/~charlesk/indicator-bluetooth/gmenuify updated
74. By Charles Kerr

add device.vala to the repo

75. By Charles Kerr

sync POTFILES.in with the new files in src/

76. By Charles Kerr

bump version to 13.10.0

77. By Charles Kerr

copyediting: don't use 'this.' when it's not needed

78. By Charles Kerr

add some extra comments

79. By Charles Kerr

copyediting: fix lines that wrap

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:79
http://jenkins.qa.ubuntu.com/job/indicator-bluetooth-ci/3/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/indicator-bluetooth-saucy-amd64-ci/2/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/indicator-bluetooth-saucy-armhf-ci/2/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/indicator-bluetooth-ci/3/rebuild

review: Needs Fixing (continuous-integration)
lp://staging/~charlesk/indicator-bluetooth/gmenuify updated
80. By Charles Kerr

add src/bluez.c, src/desktop.c, src/phone.c to POTFILES.in

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:80
http://jenkins.qa.ubuntu.com/job/indicator-bluetooth-ci/4/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-bluetooth-saucy-amd64-ci/3
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-bluetooth-saucy-armhf-ci/3

Click here to trigger a rebuild:
http://s-jenkins:8080/job/indicator-bluetooth-ci/4/rebuild

review: Approve (continuous-integration)
lp://staging/~charlesk/indicator-bluetooth/gmenuify updated
81. By Charles Kerr

templated strings need to begin with a '@'

82. By Charles Kerr

copyediting: whitespace, type inference

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:82
http://jenkins.qa.ubuntu.com/job/indicator-bluetooth-ci/5/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-bluetooth-saucy-amd64-ci/4
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-bluetooth-saucy-armhf-ci/4

Click here to trigger a rebuild:
http://s-jenkins:8080/job/indicator-bluetooth-ci/5/rebuild

review: Approve (continuous-integration)
Revision history for this message
Charles Kerr (charlesk) wrote :

bus-watch-namespace is a nice piece of work. I only have three issues -- a bug, a typo, and a question:

 * watcher->name_space is leaked

 * trivial style: ";;" used after g_cancellable_new()

 * the API contract is a little odd in that user_data can be
   destroyed before bus_unwatch_namespace() is called by the client.
   This isn't an issue in i-sound, which doesn't use the argument,
   but was it intentional in the contract?

The changes to media-player-list.vala are straightforward and fine.

review: Needs Fixing
Revision history for this message
Charles Kerr (charlesk) wrote :

...did I just paste the i-sound review into my own MP's text entry. *facepalm*

lp://staging/~charlesk/indicator-bluetooth/gmenuify updated
83. By Charles Kerr

change control s.t. either ubuntu-system-settings, or (gnome-control-center && gnome-bluetooth), are a Dependency

84. By Charles Kerr

in debian/control, make the bluez dependency explicit

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:84
http://jenkins.qa.ubuntu.com/job/indicator-bluetooth-ci/6/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-bluetooth-saucy-amd64-ci/5
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-bluetooth-saucy-armhf-ci/5

Click here to trigger a rebuild:
http://s-jenkins:8080/job/indicator-bluetooth-ci/6/rebuild

review: Approve (continuous-integration)
Revision history for this message
Mathieu Trudel-Lapierre (cyphermox) wrote :

This is huge. You got my ok re: packaging, and a cursory look at the code looks fine.

review: Approve
Revision history for this message
Ted Gould (ted) wrote :

Looks good overall. A few little things:

The indicators directory should probably be "$(datadir)/unity/indicators" instead of "$(prefix)/share"

Would be nice if the profile tracked the export of the menu and unexported it when it was destroyed. Just to clean up.

I'm not sure why the root_action isn't created with the Profile class. It seem it could just be built in the class initializer.

I don't see anywhere that Indicator3-0.4.vapi is used. Can we delete it?

review: Approve
lp://staging/~charlesk/indicator-bluetooth/gmenuify updated
85. By Charles Kerr

set indicatorsdir to $(datadir)/unity/indicators instead of $(prefix)/share/unity/indicators

86. By Charles Kerr

on shutdown, unexport the menus and unown the bus name

87. By Charles Kerr

create Profile.root_action in Profile's constructor.

88. By Charles Kerr

remove the libindicator vapi file; it's no longer used.

Revision history for this message
Charles Kerr (charlesk) wrote :

> The indicators directory should probably be "$(datadir)/unity/indicators" instead of "$(prefix)/share"

Fixed r85.

> Would be nice if the profile tracked the export of the menu and unexported it when it was destroyed. Just to clean up.

Fixed r86, though since it's destroyed when we're about to exit, it's moot...

> I'm not sure why the root_action isn't created with the Profile class. It seem it could just be built in the class initializer.

Fixed r87, nice catch.

> I don't see anywhere that Indicator3-0.4.vapi is used. Can we delete it?

Fixed r88, yes I kept it around longer than needed because for awhile I was exporting both old & new style indicators so that I could compare them side-by-side. :)

I'm leaving this unmerged for tonight in case you want to review these changes. If you're happy with them, go ahead and top-approve.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:88
http://jenkins.qa.ubuntu.com/job/indicator-bluetooth-ci/8/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-bluetooth-saucy-amd64-ci/7
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-bluetooth-saucy-armhf-ci/7

Click here to trigger a rebuild:
http://s-jenkins:8080/job/indicator-bluetooth-ci/8/rebuild

review: Approve (continuous-integration)
Revision history for this message
Ted Gould (ted) wrote :

Makes sense. I was expecting to just unexport in the destructor of the
object as Vala will clean them up as we shutdown anyway. But this is
fine, I should have been more clear.

  status approved

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches