Add D-Bus interface for battery monitoring #407
Conversation
Implements a D-Bus service (me.kavishdevar.librepods) that exposes AirPods battery information at /battery. This allows external tools like waybar to query battery levels without parsing BLE data directly. The interface exposes: - Battery levels for left, right, case, and headset - Charging status for each component - Availability status for each component - Device name and connection status - GetBatteryInfo() method returning all data as a map This enables integration with status bars and other system monitors.
📝 WalkthroughWalkthroughAdds a Qt D-Bus adaptor exposing battery/device state and a runtime D-Bus service, includes the adaptor in build files, introduces --no-tray support (propagated to TrayIconManager), and adds AirPods Pro 3 model recognition and mappings. Changes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
linux/main.cpp (1)
154-178: D-Bus initialization looks correct, but consider storing the adaptor pointer.The D-Bus registration pattern is correct: the adaptor is created as a child of
this, and thenthisis registered with the bus. Qt automatically discovers and exposes the adaptor.However, the adaptor pointer is not stored in a member variable. While Qt's parent-child relationship ensures proper cleanup, storing the pointer would make the adaptor more accessible for potential future needs (e.g., manual signal emission, testing, or dynamic updates).
💡 Optional: Store adaptor pointer for future extensibility
Consider adding a member variable:
private: QBluetoothSocket *socket = nullptr; QBluetoothSocket *phoneSocket = nullptr; + BatteryDBusAdaptor *m_dbusAdaptor = nullptr;Then in
initializeDBus():void initializeDBus() { // Create D-Bus adaptor for battery info - new BatteryDBusAdaptor(m_deviceInfo->getBattery(), m_deviceInfo, this); + m_dbusAdaptor = new BatteryDBusAdaptor(m_deviceInfo->getBattery(), m_deviceInfo, this);linux/dbusadaptor.hpp (2)
38-55: Consider removingsetAutoRelaySignals(true)as it's redundant.The
setAutoRelaySignals(true)call on line 41 is likely unnecessary since you're explicitly connecting the relevant signals to emitBatteryChanged()andDeviceChanged(). ThesetAutoRelaySignalsfeature automatically relays signals from the parent object, but since the adaptor defines its own signals and connects them manually, this setting doesn't add value and may cause confusion.🔎 Recommended refactor to remove redundant call
BatteryDBusAdaptor(Battery *battery, DeviceInfo *deviceInfo, QObject *parent) : QDBusAbstractAdaptor(parent), m_battery(battery), m_deviceInfo(deviceInfo) { - setAutoRelaySignals(true); - // Connect battery signals to our relay connect(m_battery, &Battery::batteryStatusChanged, this, [this]() { emit BatteryChanged();
84-102: Naming inconsistency between D-Bus properties and map keys.The
GetBatteryInfo()method returns a map with snake_case keys (left_level,right_level, etc.), while the D-Bus properties use PascalCase (LeftLevel,RightLevel, etc.). This inconsistency might confuse D-Bus clients who expect uniform naming conventions.Consider using consistent naming across both interfaces. Since D-Bus typically uses PascalCase for properties, you might want to use either PascalCase or snake_case consistently in the map keys as well.
💡 Example: Use consistent PascalCase naming
QVariantMap GetBatteryInfo() { QVariantMap info; - info["left_level"] = leftLevel(); - info["left_charging"] = leftCharging(); - info["left_available"] = leftAvailable(); - info["right_level"] = rightLevel(); - info["right_charging"] = rightCharging(); - info["right_available"] = rightAvailable(); - info["case_level"] = caseLevel(); - info["case_charging"] = caseCharging(); - info["case_available"] = caseAvailable(); - info["headset_level"] = headsetLevel(); - info["headset_charging"] = headsetCharging(); - info["headset_available"] = headsetAvailable(); - info["device_name"] = deviceName(); - info["connected"] = connected(); + info["LeftLevel"] = leftLevel(); + info["LeftCharging"] = leftCharging(); + info["LeftAvailable"] = leftAvailable(); + info["RightLevel"] = rightLevel(); + info["RightCharging"] = rightCharging(); + info["RightAvailable"] = rightAvailable(); + info["CaseLevel"] = caseLevel(); + info["CaseCharging"] = caseCharging(); + info["CaseAvailable"] = caseAvailable(); + info["HeadsetLevel"] = headsetLevel(); + info["HeadsetCharging"] = headsetCharging(); + info["HeadsetAvailable"] = headsetAvailable(); + info["DeviceName"] = deviceName(); + info["Connected"] = connected(); return info; }Alternatively, if snake_case is preferred for the target use case (e.g., Python integration), consider documenting this naming convention choice.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
linux/CMakeLists.txtlinux/dbusadaptor.hpplinux/main.cpp
🧰 Additional context used
🧬 Code graph analysis (2)
linux/main.cpp (1)
linux/dbusadaptor.hpp (2)
m_deviceInfo(76-76)m_deviceInfo(77-80)
linux/dbusadaptor.hpp (2)
linux/deviceinfo.hpp (2)
leftLevel(195-206)deviceNameChanged(97-97)linux/main.cpp (3)
m_deviceInfo(139-139)m_deviceInfo(141-141)m_deviceInfo(154-178)
🔇 Additional comments (4)
linux/CMakeLists.txt (1)
44-44: LGTM!The addition of
dbusadaptor.hppto the sources is correct and necessary for Qt's Meta-Object Compiler (MOC) to process the D-Bus adaptor class.linux/main.cpp (1)
33-34: LGTM!The necessary D-Bus headers are correctly included to support the new D-Bus integration.
linux/dbusadaptor.hpp (2)
10-35: LGTM! Well-structured D-Bus interface definition.The D-Bus interface is well-designed with clear property declarations covering battery levels, charging status, availability, and device information. The use of
Q_CLASSINFOto specify the interface name follows Qt D-Bus conventions.
77-80: Connection status logic looks reasonable.The
connected()method checks if the device name is set and at least one pod (left, right, or headset) is available. Not checkingcaseAvailable()makes sense since the case being available alone doesn't indicate that AirPods are actively connected to the system.
- Add AirPodsPro3 to AirPodsModel enum - Add model number mappings (A3063, A3064, A3065) - Add Bluetooth model ID (0x3F20) for BLE detection - Update icon mappings to use AirPods Pro icons
Discovered via BLE scanning that AirPods Pro 3 can also broadcast with model ID 0x2720 in addition to 0x3F20. This ensures the model is correctly identified regardless of which ID is used.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
linux/ble/blemanager.cpp (1)
21-23: LGTM! The dual model ID mapping for AirPods Pro 3 is correctly implemented.The addition of both 0x2720 and 0x3F20 model IDs mapping to
AirPodsModel::AirPodsPro3is sound. TheAirPodsPro3enum value exists in the enum definition.For consistency with the other entries in the map, consider adding a trailing comma to line 23:
- {0x3F20, AirPodsModel::AirPodsPro3} + {0x3F20, AirPodsModel::AirPodsPro3},
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
linux/ble/blemanager.cpp
🧰 Additional context used
🪛 Cppcheck (2.19.0)
linux/ble/blemanager.cpp
[error] 22-22: There is an unknown macro here somewhere. Configuration is required. If Q_ENUM_NS is a macro then please configure it.
(unknownMacro)
Useful when using external status bars (like waybar) that query battery info via D-Bus instead of relying on the tray icon. Usage: librepods --hide --no-tray
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
linux/trayiconmanager.cpp (1)
14-30: Consider deferring resource allocation when noTray is true.Currently, the tray icon object, menu, and signal connections are created even when
noTrayis true (the icon is simply not shown). This allocates resources that won't be used. Consider deferring or skipping this initialization when the tray is disabled for better resource efficiency.Potential optimization approach
TrayIconManager::TrayIconManager(QObject *parent, bool noTray) : QObject(parent), m_noTray(noTray) { - // Initialize tray icon - trayIcon = new QSystemTrayIcon(QIcon(":/icons/assets/airpods.png"), this); - trayMenu = new QMenu(); - - // Setup basic menu actions - setupMenuActions(); - - // Connect signals - trayIcon->setContextMenu(trayMenu); - connect(trayIcon, &QSystemTrayIcon::activated, this, &TrayIconManager::onTrayIconActivated); - - // Only show tray icon if not disabled if (!noTray) { + // Initialize tray icon + trayIcon = new QSystemTrayIcon(QIcon(":/icons/assets/airpods.png"), this); + trayMenu = new QMenu(); + + // Setup basic menu actions + setupMenuActions(); + + // Connect signals + trayIcon->setContextMenu(trayMenu); + connect(trayIcon, &QSystemTrayIcon::activated, this, &TrayIconManager::onTrayIconActivated); + trayIcon->show(); + } else { + trayIcon = nullptr; + trayMenu = nullptr; } }Note: This would require null checks in methods like
updateBatteryStatus(),showNotification(), etc.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
linux/main.cpplinux/trayiconmanager.cpplinux/trayiconmanager.h
🧰 Additional context used
🧬 Code graph analysis (3)
linux/main.cpp (1)
linux/dbusadaptor.hpp (2)
m_deviceInfo(76-76)m_deviceInfo(77-80)
linux/trayiconmanager.h (1)
linux/trayiconmanager.cpp (1)
TrayIconManager(14-31)
linux/trayiconmanager.cpp (1)
linux/trayiconmanager.h (1)
TrayIconManager(10-66)
🔇 Additional comments (11)
linux/trayiconmanager.h (2)
16-16: LGTM! Backward-compatible constructor update.The addition of the
noTrayparameter with a default value maintains backward compatibility while enabling the new no-tray mode.
54-54: LGTM! Private member added correctly.The
m_noTraymember follows naming conventions and is properly initialized with a default value.linux/trayiconmanager.cpp (2)
14-14: LGTM! Constructor properly initializes the no-tray state.The constructor correctly accepts and initializes the
noTrayparameter via the member initializer list.
27-30: LGTM! Tray icon visibility correctly controlled.The conditional logic ensures the tray icon is only shown when
noTrayis false, implementing the expected behavior.linux/main.cpp (7)
33-34: LGTM! Required includes added.The D-Bus adaptor header and QDBusConnection are correctly included for the new D-Bus integration.
54-56: LGTM! Constructor signature extended to support no-tray mode.The
noTrayparameter is properly added with a default value and correctly initialized via the member initializer list.
63-64: LGTM! No-tray parameter correctly propagated.The
noTrayflag is properly passed to theTrayIconManagerconstructor, enabling the tray icon to be conditionally hidden.
1011-1011: LGTM! Private member added for no-tray state.The
m_noTraymember correctly tracks the no-tray mode state.
1064-1074: LGTM! Command-line argument parsing for --no-tray.The
--no-trayoption is correctly parsed and the flag is set appropriately.
1079-1079: LGTM! No-tray parameter passed to app instance.The
noTrayflag is correctly passed to theAirPodsTrayAppconstructor.
119-120: No action required. The D-Bus initialization ordering is correct.The code already safely handles uninitialized device state. Battery is initialized with default values (level=0, status=Disconnected) in its constructor, and BatteryDBusAdaptor gracefully returns safe defaults when device data is unavailable: battery levels return 0, availability returns false, and the
connected()property returns false if the device name is empty or no components are available. D-Bus registration ininitializeDBus()only registers the adaptor and signals; it does not perform any data reads that depend on Bluetooth initialization. Device data is properly populated later ininitializeBluetooth()via settings loading and BLE scan.Likely an incorrect or invalid review comment.
| void initializeDBus() { | ||
| // Create D-Bus adaptor for battery info | ||
| new BatteryDBusAdaptor(m_deviceInfo->getBattery(), m_deviceInfo, this); | ||
|
|
||
| // Register on session bus | ||
| QDBusConnection sessionBus = QDBusConnection::sessionBus(); | ||
| if (!sessionBus.isConnected()) { | ||
| LOG_ERROR("Cannot connect to D-Bus session bus"); | ||
| return; | ||
| } | ||
|
|
||
| // Register service | ||
| if (!sessionBus.registerService("me.kavishdevar.librepods")) { | ||
| LOG_ERROR("Cannot register D-Bus service: " << sessionBus.lastError().message()); | ||
| return; | ||
| } | ||
|
|
||
| // Register object | ||
| if (!sessionBus.registerObject("/battery", this)) { | ||
| LOG_ERROR("Cannot register D-Bus object: " << sessionBus.lastError().message()); | ||
| return; | ||
| } | ||
|
|
||
| LOG_INFO("D-Bus service registered: me.kavishdevar.librepods at /battery"); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for D-Bus service name conflicts and registration patterns
# Check if BatteryDBusAdaptor follows Qt's parent-managed pattern
echo "=== Checking BatteryDBusAdaptor implementation ==="
rg -n "class BatteryDBusAdaptor" -A 20
# Check for any cleanup or unregister logic
echo -e "\n=== Checking for D-Bus cleanup/unregister logic ==="
rg -n "unregisterService|unregisterObject" --type cpp
# Check how other parts of the code handle single-instance scenarios
echo -e "\n=== Checking single-instance handling ==="
rg -n "QLocalServer|instance.*running" -C 3 --type cppRepository: kavishdevar/librepods
Length of output: 4236
🏁 Script executed:
#!/bin/bash
# Check BatteryDBusAdaptor constructor to confirm parent pattern
echo "=== BatteryDBusAdaptor constructor ==="
rg -n "BatteryDBusAdaptor::" -A 5
# Check for destructors or cleanup code
echo -e "\n=== Checking destructors/cleanup ==="
rg -n "~.*Application|~.*MainWindow|~.*TrayApplication" -A 10 --type cpp
# Check if there's any shutdown handling for D-Bus
echo -e "\n=== Checking application shutdown/exit code ==="
rg -n "aboutToQuit|destroyed|closeEvent" -B 2 -A 5 --type cpp
# Check the full initializeDBus function context
echo -e "\n=== Full D-Bus initialization context ==="
sed -n '150,180p' linux/main.cppRepository: kavishdevar/librepods
Length of output: 1716
🏁 Script executed:
#!/bin/bash
# Check full BatteryDBusAdaptor header definition
echo "=== Full BatteryDBusAdaptor header ==="
cat -n linux/dbusadaptor.hppRepository: kavishdevar/librepods
Length of output: 5212
Add D-Bus cleanup on registration failures and application shutdown.
The D-Bus initialization code is mostly correct. The BatteryDBusAdaptor memory management is proper—it correctly uses Qt's parent-child pattern by passing this to QDBusAbstractAdaptor as a parent (line 38-39).
However, there are two issues to address:
-
Partial initialization cleanup missing: If service registration succeeds (line 166) but object registration fails (line 172), the service remains registered without an object. Consider calling
sessionBus.unregisterService()before returning on object registration failure. -
No D-Bus cleanup on shutdown: The
aboutToQuithandler (line 1142-1147) only closes the local server but doesn't unregister the D-Bus service and object. Add calls tosessionBus.unregisterObject()andsessionBus.unregisterService()to properly clean up on shutdown.
Service name conflicts are mitigated by the existing single-instance mechanism (via QLocalServer), which prevents concurrent instances from running.
🤖 Prompt for AI Agents
In linux/main.cpp around lines 154-178, the D-Bus init routine currently
registers the service and object but fails to clean up if object registration
fails; update the failure path to call
sessionBus.unregisterService("me.kavishdevar.librepods") (or unregister the
service name you registered) before returning when registerObject() fails so the
service isn't left registered alone. Also update the aboutToQuit handler around
lines 1142-1147 to explicitly call
QDBusConnection::sessionBus().unregisterObject("/battery") and
QDBusConnection::sessionBus().unregisterService("me.kavishdevar.librepods")
(guarded if necessary) so the D-Bus object and service are unregistered on
shutdown.
1 participant
Summary
This PR implements a D-Bus interface that exposes AirPods battery information, enabling integration with status bars (like waybar) and other system monitoring tools.
Changes
dbusadaptor.hpp- D-Bus adaptor class that exposes battery propertiesinitializeDBus()inmain.cppto register the D-Bus servicedbusadaptor.hpptoCMakeLists.txtD-Bus Interface
Service:
me.kavishdevar.librepodsObject Path:
/batteryInterface:
me.kavishdevar.librepods.BatteryProperties
LeftLevel,RightLevel,CaseLevel,HeadsetLevel(int)LeftCharging,RightCharging,CaseCharging,HeadsetCharging(bool)LeftAvailable,RightAvailable,CaseAvailable,HeadsetAvailable(bool)DeviceName(string)Connected(bool)Methods
GetBatteryInfo()- Returns all battery info as a mapExample Usage
# Query all battery info dbus-send --session --print-reply --dest=me.kavishdevar.librepods \ /battery me.kavishdevar.librepods.Battery.GetBatteryInfoWaybar Integration
Example Python script for waybar:
Test Plan
GetBatteryInfo()returns correct battery levelsSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.