Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement a new Bluetooth indicator #459

Merged
merged 81 commits into from
Jan 27, 2024

Conversation

EbonJaeger
Copy link
Member

@EbonJaeger EbonJaeger commented Sep 23, 2023

Description

The Bluetooth rewrite is finally here! GNOME Bluetooth is entirely dropped from Budgie Desktop; we now use Bluez and UPower directly, giving us full control over how Bluetooth is interacted with. You will now be able to connect to and disconnect from your Bluetooth devices from the panel without having to go to the Control Center. You will still have to visit the Control Center to pair new devices, however. With the direct usage of UPower, you will also see the battery power of devices that report it right in the applet popover.

This also includes our own implementation of Sendto in order to send and receive files over Bluetooth. Budgie Sendto is very heavily inspired by Elementary's own Bluetooth Sendto implementation. It runs in the background to listen for file transfer requests from other devices, showing a notification when one is received, enabling you to accept or reject the transfer. To send a file, you can open the Bluetooth indicator popover and hit the Send button to open a dialog to choose what device you want to send to.

Closes #197

Submitter Checklist

  • Squashed commits with git rebase -i (if needed)
  • Built budgie-desktop and verified that the patch worked (if needed)

@EbonJaeger EbonJaeger added the enhancement New feature or request label Sep 23, 2023
@EbonJaeger EbonJaeger added this to the 10.9 milestone Sep 23, 2023
@EbonJaeger EbonJaeger force-pushed the 197-implement-a-new-bluetooth-item branch from bd40752 to 52e379e Compare September 23, 2023 14:46
@fossfreedom
Copy link
Contributor

fossfreedom commented Dec 26, 2023

FYI - Debian has now marked the usage of libgnome-bluetooth13 as something to be removed before its next release since upstream has dropped support.

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1030130 and https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1059486

So great news that we have this PR - nice one @EbonJaeger

This also dropping support from BCC as well - https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1059485 ...

but with the need to pair devices via BCC that's more problematic - we would have to rewrite this whole section in BCC ... either with the suggested new gnome library ... or a complete rewrite to use bluez?

EDIT: or maybe less elegant but very little work - just drop bluetooth from BCC entirely - and add here a callout to blueman to-do the GUI pairing aspects?

Copy link
Member

@JoshStrobl JoshStrobl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Been using Bluetooth applet and it connects / disconnects just fine with my Fairbuds XL headphones. Battery constantly reports 100% but I'm leaning on that being more an issue with upower than anything else. I'll re-install d-spy and take a look.

Some feedback provided mainly on sendto, some interesting logic for formatting time and duplication that should ideally be avoided.

Design feedback: Maybe could do with some padding around the edges of the popover contents. Night Light and Sound Indicator are a couple examples, but the padding may only really be visible (excluding Night Light header where it's hardcoded into the widget code) when using Materia. Not sure about other themes when they are used in conjunction with the internal theme.

Overall, fantastic work. I can see a lot of care and attention went into this.

src/dialogs/sendto/Dialog/FileReceiver.vala Outdated Show resolved Hide resolved
src/dialogs/sendto/Dialog/FileSender.vala Outdated Show resolved Hide resolved
src/dialogs/sendto/Dialog/ScanDialog.vala Outdated Show resolved Hide resolved
src/dialogs/sendto/Services/Manager.vala Outdated Show resolved Hide resolved
src/dialogs/sendto/Services/Manager.vala Show resolved Hide resolved
src/dialogs/sendto/Services/Manager.vala Show resolved Hide resolved
src/panel/applets/status/BluetoothIndicator.vala Outdated Show resolved Hide resolved
@JoshStrobl JoshStrobl force-pushed the 197-implement-a-new-bluetooth-item branch from 52e379e to fc3e00d Compare January 10, 2024 20:45
@JoshStrobl
Copy link
Member

Rebased it, ignore noise on the billion commits after my suggestions :D

@EbonJaeger
Copy link
Member Author

I was wondering why some of the review items were showing "outdated" xD

@EbonJaeger
Copy link
Member Author

The latest commit addresses the duplicated code and estimated time calculation. If you could test receiving a file via Bluetooth, that would be great. I'm not getting the notification to reject or accept the transfer, and I have no idea if it's the code, my phone, or the deity of Bluetooth deciding to mess with me.

@JoshStrobl
Copy link
Member

@EbonJaeger I am able to send files from my desktop to the test phone just fine but sending from my test phone to the desktop, getting the notification to accept and accepting it, does not work. Just to be sure, I tested it with a unique file not on my desktop.

Also as a note, we need a newline between the From: FILEPATH and "Sent to: DEVICE". Otherwise it all melds into one line.

Will continue testing but hopefully @fossfreedom can do some further testing on his end too since he uses sendto functionality (or at the very least I know he certainly does with more frequency than me -- which is never :D)


Otherwise, LGTM. Send-to dialog looked good, though we may want to consider filtering out devices you really can't send to like headphones and wireless keyboards.

@fossfreedom
Copy link
Contributor

some quick thoughts.

  1. with bluetooth disabled the popover has both a bluetooth settings button in the top section and a big bluetooth button in the popover body. The latter is perhaps superfluous

  2. similarly with nothing paired the second button is there.
    image

  3. the send-to button is enabled even though my phone is marked as disconnected. Can this be disabled until a bluetooth device is connected?
    image

image

in BCC you click on a connected device and then you send a file directly.
in the popover you click the send files in the top of the popover that then displays this

image

and then you have a whirring discovery option at the top of the dialog and a separate "send" button on the device to send to.

I'm pretty sure this was discussed in #general ... but too far to scroll back - did we discount having a send button next to the disconnect option to directly send to the chosen device - i.e. one click like BCC rather than two clicks via the popover to send a file ?

  1. disconnect buttons look a little vertically stretched

image

Tested with Qogir, Arc and Pocillo

  1. in that screenshot the second button (Qogir) has a border around the button. Is this trying to say something specifically about this disconnect button? Or should both phones disconnect buttons have borders so that the buttons look like buttons?

Tested with a couple of connected phones sending files back and forth. works nicely

@EbonJaeger
Copy link
Member Author

@fossfreedom

  1. & 2. Good point. Removed the button in the placeholder
  2. Good idea. Done. (Superseded by below)
  3. Ended up being way easier to do than I thought. Devices that support file sending (desktops, laptops, and smartphones; are there any others in this list?) now have their own individual send button that bypasses the first dialog.
  4. Still can't get this to happen :/
  5. I've noticed that sometimes, buttons don't quite follow their set relief style. Been this way for a while, now.

@fossfreedom
Copy link
Contributor

4. Still can't get this to happen :/

hmm - I'm wonder if the send and disconnect buttons should be added to a buttonbox rather than pack-start within a box?

If I connect to a phone via the popover the disconnect button appears in the popover. Good. However the send button doesn't appear. If I restart the panel then both the send button and disconnect buttons appear in the popover.

@EbonJaeger
Copy link
Member Author

@fossfreedom I think I might have it this time. Can you try the latest commit?

@fossfreedom
Copy link
Contributor

Nice work! Relief issues have been resolved.

Connected my phone - send files button enabled.

Connected my airpods - connected - no send files button - so thats good news.

@fossfreedom
Copy link
Contributor

good to be merged IMHO.

(I hope... The organization is a mess.)

Signed-off-by: Evan Maddock <[email protected]>
Signed-off-by: Evan Maddock <[email protected]>
Signed-off-by: Evan Maddock <[email protected]>
Signed-off-by: Evan Maddock <[email protected]>
At this time, I have no idea how to integrate the UPower aspect with the Bluetooth devices. Moreover, I can't test if it's working even if I did, because none of my Bluetooth devices trigger the UPower signals.

Signed-off-by: Evan Maddock <[email protected]>
It'll automagically figure it out for DBus.

Signed-off-by: Evan Maddock <[email protected]>
Signed-off-by: Evan Maddock <[email protected]>
EbonJaeger and others added 27 commits January 27, 2024 17:00
Signed-off-by: Evan Maddock <[email protected]>
Also fixes saving received files to the Downloads folder.

Signed-off-by: Evan Maddock <[email protected]>
This means that the sending device will no longer still send the file, even if the transfer was rejected.

Signed-off-by: Evan Maddock <[email protected]>
Signed-off-by: Evan Maddock <[email protected]>
Both the send and receive dialogs are almost the same, the main difference being in the background implementation and wording of strings. This causes some duplicated logic, especially around the formatting of times remaining.

This adds a new base class that both dialogs can derive from while still having their own implementations for doing the work, leading to less duplicated code.

Signed-off-by: Evan Maddock <[email protected]>
@JoshStrobl JoshStrobl force-pushed the 197-implement-a-new-bluetooth-item branch from 76ea84e to 891839e Compare January 27, 2024 15:00
@JoshStrobl JoshStrobl merged commit 37d4f28 into main Jan 27, 2024
1 check failed
@JoshStrobl JoshStrobl deleted the 197-implement-a-new-bluetooth-item branch January 27, 2024 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Implement a new Bluetooth item
3 participants