Commit d5b47048 authored by Sonny Sasaka's avatar Sonny Sasaka Committed by Commit Bot

device/bluetooth: Add BluetoothBatteryClient

This CL adds BluetoothBatteryClient which is a proxy to BlueZ's
org.bluez.Battery1 interface. Future CLs will interact with this new
interface to get battery level of Bluetooth devices and store the
value to BluetoothDevice object.

Battery API Doc:
https://chromium.googlesource.com/chromiumos/third_party/bluez/+/refs/heads/chromeos-5.54/doc/battery-api.txt

Bug: b:160905767
Change-Id: I2bee100b25518b48a296b87657026769058384d3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2296266
Commit-Queue: Sonny Sasaka <sonnysasaka@chromium.org>
Reviewed-by: default avatarRyan Hansberry <hansberry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#789676}
parent f1185cf2
......@@ -324,6 +324,8 @@ component("bluetooth") {
"dbus/bluetooth_agent_manager_client.h",
"dbus/bluetooth_agent_service_provider.cc",
"dbus/bluetooth_agent_service_provider.h",
"dbus/bluetooth_battery_client.cc",
"dbus/bluetooth_battery_client.h",
"dbus/bluetooth_dbus_client_bundle.cc",
"dbus/bluetooth_dbus_client_bundle.h",
"dbus/bluetooth_debug_manager_client.cc",
......@@ -400,6 +402,8 @@ component("bluetooth") {
"dbus/fake_bluetooth_agent_manager_client.h",
"dbus/fake_bluetooth_agent_service_provider.cc",
"dbus/fake_bluetooth_agent_service_provider.h",
"dbus/fake_bluetooth_battery_client.cc",
"dbus/fake_bluetooth_battery_client.h",
"dbus/fake_bluetooth_debug_manager_client.cc",
"dbus/fake_bluetooth_debug_manager_client.h",
"dbus/fake_bluetooth_device_client.cc",
......
......@@ -40,6 +40,7 @@
#include "device/bluetooth/dbus/bluetooth_adapter_client.h"
#include "device/bluetooth/dbus/bluetooth_agent_manager_client.h"
#include "device/bluetooth/dbus/bluetooth_agent_service_provider.h"
#include "device/bluetooth/dbus/bluetooth_battery_client.h"
#include "device/bluetooth/dbus/bluetooth_device_client.h"
#include "device/bluetooth/dbus/bluetooth_gatt_application_service_provider.h"
#include "device/bluetooth/dbus/bluetooth_gatt_manager_client.h"
......@@ -244,6 +245,8 @@ void BluetoothAdapterBlueZ::Shutdown() {
bluez::BluezDBusManager::Get()->GetBluetoothAdapterClient()->RemoveObserver(
this);
bluez::BluezDBusManager::Get()->GetBluetoothBatteryClient()->RemoveObserver(
this);
bluez::BluezDBusManager::Get()->GetBluetoothDeviceClient()->RemoveObserver(
this);
bluez::BluezDBusManager::Get()->GetBluetoothInputClient()->RemoveObserver(
......@@ -282,6 +285,8 @@ void BluetoothAdapterBlueZ::Init() {
bluez::BluezDBusManager::Get()->GetBluetoothAdapterClient()->AddObserver(
this);
bluez::BluezDBusManager::Get()->GetBluetoothBatteryClient()->AddObserver(
this);
bluez::BluezDBusManager::Get()->GetBluetoothDeviceClient()->AddObserver(this);
bluez::BluezDBusManager::Get()->GetBluetoothInputClient()->AddObserver(this);
bluez::BluezDBusManager::Get()->GetBluetoothAgentManagerClient()->AddObserver(
......@@ -641,6 +646,21 @@ void BluetoothAdapterBlueZ::AdapterPropertyChanged(
}
}
void BluetoothAdapterBlueZ::BatteryAdded(const dbus::ObjectPath& object_path) {
// TODO(b/160905767): Handle it by updating device battery percentage field.
}
void BluetoothAdapterBlueZ::BatteryRemoved(
const dbus::ObjectPath& object_path) {
// TODO(b/160905767): Handle it by updating device battery percentage field.
}
void BluetoothAdapterBlueZ::BatteryPropertyChanged(
const dbus::ObjectPath& object_path,
const std::string& property_name) {
// TODO(b/160905767): Handle it by updating device battery percentage field.
}
void BluetoothAdapterBlueZ::DeviceAdded(const dbus::ObjectPath& object_path) {
DCHECK(bluez::BluezDBusManager::Get());
bluez::BluetoothDeviceClient::Properties* properties =
......
......@@ -28,6 +28,7 @@
#include "device/bluetooth/dbus/bluetooth_adapter_client.h"
#include "device/bluetooth/dbus/bluetooth_agent_manager_client.h"
#include "device/bluetooth/dbus/bluetooth_agent_service_provider.h"
#include "device/bluetooth/dbus/bluetooth_battery_client.h"
#include "device/bluetooth/dbus/bluetooth_device_client.h"
#include "device/bluetooth/dbus/bluetooth_input_client.h"
#include "device/bluetooth/dbus/bluetooth_profile_manager_client.h"
......@@ -77,6 +78,7 @@ class BluetoothPairingBlueZ;
class DEVICE_BLUETOOTH_EXPORT BluetoothAdapterBlueZ final
: public device::BluetoothAdapter,
public bluez::BluetoothAdapterClient::Observer,
public bluez::BluetoothBatteryClient::Observer,
public bluez::BluetoothDeviceClient::Observer,
public bluez::BluetoothInputClient::Observer,
public bluez::BluetoothAgentManagerClient::Observer,
......@@ -287,6 +289,12 @@ class DEVICE_BLUETOOTH_EXPORT BluetoothAdapterBlueZ final
void AdapterPropertyChanged(const dbus::ObjectPath& object_path,
const std::string& property_name) override;
// bluez::BluetoothBatteryClient::Observer override.
void BatteryAdded(const dbus::ObjectPath& object_path) override;
void BatteryRemoved(const dbus::ObjectPath& object_path) override;
void BatteryPropertyChanged(const dbus::ObjectPath& object_path,
const std::string& property_name) override;
// bluez::BluetoothDeviceClient::Observer override.
void DeviceAdded(const dbus::ObjectPath& object_path) override;
void DeviceRemoved(const dbus::ObjectPath& object_path) override;
......
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "device/bluetooth/dbus/bluetooth_battery_client.h"
#include "base/bind.h"
#include "base/logging.h"
#include "base/macros.h"
#include "base/memory/ptr_util.h"
#include "base/stl_util.h"
#include "dbus/bus.h"
#include "dbus/message.h"
#include "dbus/object_manager.h"
#include "dbus/object_proxy.h"
#include "third_party/cros_system_api/dbus/service_constants.h"
namespace bluez {
BluetoothBatteryClient::Properties::Properties(
dbus::ObjectProxy* object_proxy,
const std::string& interface_name,
const PropertyChangedCallback& callback)
: dbus::PropertySet(object_proxy, interface_name, callback) {
RegisterProperty(bluetooth_battery::kPercentageProperty, &percentage);
}
BluetoothBatteryClient::Properties::~Properties() = default;
// The BluetoothBatteryClient implementation used in production.
class BluetoothBatteryClientImpl : public BluetoothBatteryClient,
public dbus::ObjectManager::Interface {
public:
BluetoothBatteryClientImpl() = default;
~BluetoothBatteryClientImpl() override {
// There is an instance of this client that is created but not initialized
// on Linux. See 'Alternate D-Bus Client' note in bluez_dbus_manager.h.
if (object_manager_) {
object_manager_->UnregisterInterface(
bluetooth_adapter::kBluetoothAdapterInterface);
}
}
// BluetoothBatteryClient override.
void AddObserver(BluetoothBatteryClient::Observer* observer) override {
DCHECK(observer);
observers_.AddObserver(observer);
}
// BluetoothBatteryClient override.
void RemoveObserver(BluetoothBatteryClient::Observer* observer) override {
DCHECK(observer);
observers_.RemoveObserver(observer);
}
// dbus::ObjectManager::Interface override.
dbus::PropertySet* CreateProperties(
dbus::ObjectProxy* object_proxy,
const dbus::ObjectPath& object_path,
const std::string& interface_name) override {
return new Properties(
object_proxy, interface_name,
base::BindRepeating(&BluetoothBatteryClientImpl::OnPropertyChanged,
weak_ptr_factory_.GetWeakPtr(), object_path));
}
// BluetoothBatteryClient override.
std::vector<dbus::ObjectPath> GetBatteriesForAdapter(
const dbus::ObjectPath& adapter_path) override {
std::vector<dbus::ObjectPath> object_paths, battery_paths;
battery_paths = object_manager_->GetObjectsWithInterface(
bluetooth_battery::kBluetoothBatteryInterface);
for (const auto& path : battery_paths)
object_paths.push_back(path);
return object_paths;
}
// BluetoothBatteryClient override.
Properties* GetProperties(const dbus::ObjectPath& object_path) override {
return static_cast<Properties*>(object_manager_->GetProperties(
object_path, bluetooth_battery::kBluetoothBatteryInterface));
}
protected:
void Init(dbus::Bus* bus,
const std::string& bluetooth_service_name) override {
object_manager_ = bus->GetObjectManager(
bluetooth_service_name,
dbus::ObjectPath(
bluetooth_object_manager::kBluetoothObjectManagerServicePath));
object_manager_->RegisterInterface(
bluetooth_battery::kBluetoothBatteryInterface, this);
}
private:
// Called by dbus::ObjectManager when an object with the battery interface
// is created. Informs observers.
void ObjectAdded(const dbus::ObjectPath& object_path,
const std::string& interface_name) override {
for (auto& observer : observers_)
observer.BatteryAdded(object_path);
}
// Called by dbus::ObjectManager when an object with the battery interface
// is removed. Informs observers.
void ObjectRemoved(const dbus::ObjectPath& object_path,
const std::string& interface_name) override {
for (auto& observer : observers_)
observer.BatteryRemoved(object_path);
}
// Called by BluetoothPropertySet when a property value is changed,
// either by result of a signal or response to a GetAll() or Get()
// call. Informs observers.
void OnPropertyChanged(const dbus::ObjectPath& object_path,
const std::string& property_name) {
for (auto& observer : observers_)
observer.BatteryPropertyChanged(object_path, property_name);
}
dbus::ObjectManager* object_manager_ = nullptr;
// List of observers interested in event notifications from us.
base::ObserverList<BluetoothBatteryClient::Observer>::Unchecked observers_;
// Weak pointer factory for generating 'this' pointers that might live longer
// than we do.
// Note: This should remain the last member so it'll be destroyed and
// invalidate its weak pointers before any other members are destroyed.
base::WeakPtrFactory<BluetoothBatteryClientImpl> weak_ptr_factory_{this};
DISALLOW_COPY_AND_ASSIGN(BluetoothBatteryClientImpl);
};
BluetoothBatteryClient::BluetoothBatteryClient() = default;
BluetoothBatteryClient::~BluetoothBatteryClient() = default;
BluetoothBatteryClient* BluetoothBatteryClient::Create() {
return new BluetoothBatteryClientImpl();
}
} // namespace bluez
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef DEVICE_BLUETOOTH_DBUS_BLUETOOTH_BATTERY_CLIENT_H_
#define DEVICE_BLUETOOTH_DBUS_BLUETOOTH_BATTERY_CLIENT_H_
#include <stdint.h>
#include <string>
#include <vector>
#include "base/callback.h"
#include "base/macros.h"
#include "base/observer_list.h"
#include "base/values.h"
#include "dbus/object_path.h"
#include "dbus/property.h"
#include "device/bluetooth/bluetooth_export.h"
#include "device/bluetooth/dbus/bluez_dbus_client.h"
namespace bluez {
// BluetoothBatteryClient is used to communicate with objects on BlueZ's
// org.bluez.Battery1 interface.
class DEVICE_BLUETOOTH_EXPORT BluetoothBatteryClient : public BluezDBusClient {
public:
// Structure of properties on org.bluez.Battery1 interface.
struct Properties : public dbus::PropertySet {
// The percentage (0-100) of the battery. Read-only.
dbus::Property<uint8_t> percentage;
Properties(dbus::ObjectProxy* object_proxy,
const std::string& interface_name,
const PropertyChangedCallback& callback);
~Properties() override;
};
// Interface for observing changes from a remote bluetooth battery.
class Observer {
public:
virtual ~Observer() = default;
// Called when the remote battery with object path |object_path| is added
// to the set of known batteries.
virtual void BatteryAdded(const dbus::ObjectPath& object_path) {}
// Called when the remote battery with object path |object_path| is removed
// from the set of known batteries.
virtual void BatteryRemoved(const dbus::ObjectPath& object_path) {}
// Called when the battery with object path |object_path| has a
// change in value of the property named |property_name|.
virtual void BatteryPropertyChanged(const dbus::ObjectPath& object_path,
const std::string& property_name) {}
};
~BluetoothBatteryClient() override;
// Adds and removes observers for events on all remote bluetooth
// batteries. Check the |object_path| parameter of observer methods to
// determine which battery is issuing the event.
virtual void AddObserver(Observer* observer) = 0;
virtual void RemoveObserver(Observer* observer) = 0;
// Returns the list of battery object paths associated with the given adapter
// identified by the D-Bus object path |adapter_path|.
virtual std::vector<dbus::ObjectPath> GetBatteriesForAdapter(
const dbus::ObjectPath& adapter_path) = 0;
// Obtain the properties for the battery with object path |object_path|,
// any values should be copied if needed.
virtual Properties* GetProperties(const dbus::ObjectPath& object_path) = 0;
// Creates the instance.
static BluetoothBatteryClient* Create();
protected:
BluetoothBatteryClient();
private:
DISALLOW_COPY_AND_ASSIGN(BluetoothBatteryClient);
};
} // namespace bluez
#endif // DEVICE_BLUETOOTH_DBUS_BLUETOOTH_BATTERY_CLIENT_H_
......@@ -12,6 +12,7 @@
#include "base/strings/string_util.h"
#include "device/bluetooth/dbus/bluetooth_adapter_client.h"
#include "device/bluetooth/dbus/bluetooth_agent_manager_client.h"
#include "device/bluetooth/dbus/bluetooth_battery_client.h"
#include "device/bluetooth/dbus/bluetooth_debug_manager_client.h"
#include "device/bluetooth/dbus/bluetooth_device_client.h"
#include "device/bluetooth/dbus/bluetooth_gatt_characteristic_client.h"
......@@ -23,6 +24,7 @@
#include "device/bluetooth/dbus/bluetooth_profile_manager_client.h"
#include "device/bluetooth/dbus/fake_bluetooth_adapter_client.h"
#include "device/bluetooth/dbus/fake_bluetooth_agent_manager_client.h"
#include "device/bluetooth/dbus/fake_bluetooth_battery_client.h"
#include "device/bluetooth/dbus/fake_bluetooth_debug_manager_client.h"
#include "device/bluetooth/dbus/fake_bluetooth_device_client.h"
#include "device/bluetooth/dbus/fake_bluetooth_gatt_characteristic_client.h"
......@@ -43,6 +45,7 @@ BluetoothDBusClientBundle::BluetoothDBusClientBundle(bool use_fakes)
BluetoothLEAdvertisingManagerClient::Create());
bluetooth_agent_manager_client_.reset(
BluetoothAgentManagerClient::Create());
bluetooth_battery_client_.reset(BluetoothBatteryClient::Create());
bluetooth_debug_manager_client_.reset(
BluetoothDebugManagerClient::Create());
bluetooth_device_client_.reset(BluetoothDeviceClient::Create());
......@@ -66,6 +69,7 @@ BluetoothDBusClientBundle::BluetoothDBusClientBundle(bool use_fakes)
bluetooth_le_advertising_manager_client_.reset(
new FakeBluetoothLEAdvertisingManagerClient);
bluetooth_agent_manager_client_.reset(new FakeBluetoothAgentManagerClient);
bluetooth_battery_client_.reset(new FakeBluetoothBatteryClient);
bluetooth_debug_manager_client_.reset(new FakeBluetoothDebugManagerClient);
bluetooth_device_client_.reset(new FakeBluetoothDeviceClient);
bluetooth_input_client_.reset(new FakeBluetoothInputClient);
......
......@@ -15,6 +15,7 @@ namespace bluez {
class BluetoothAdapterClient;
class BluetoothAgentManagerClient;
class BluetoothBatteryClient;
class BluetoothDebugManagerClient;
class BluetoothDeviceClient;
class BluetoothGattCharacteristicClient;
......@@ -53,6 +54,10 @@ class DEVICE_BLUETOOTH_EXPORT BluetoothDBusClientBundle {
return bluetooth_debug_manager_client_.get();
}
BluetoothBatteryClient* bluetooth_battery_client() {
return bluetooth_battery_client_.get();
}
BluetoothDeviceClient* bluetooth_device_client() {
return bluetooth_device_client_.get();
}
......@@ -98,6 +103,7 @@ class DEVICE_BLUETOOTH_EXPORT BluetoothDBusClientBundle {
std::unique_ptr<BluetoothLEAdvertisingManagerClient>
bluetooth_le_advertising_manager_client_;
std::unique_ptr<BluetoothAgentManagerClient> bluetooth_agent_manager_client_;
std::unique_ptr<BluetoothBatteryClient> bluetooth_battery_client_;
std::unique_ptr<BluetoothDebugManagerClient> bluetooth_debug_manager_client_;
std::unique_ptr<BluetoothDeviceClient> bluetooth_device_client_;
std::unique_ptr<BluetoothGattCharacteristicClient>
......
......@@ -23,6 +23,7 @@
#include "device/base/features.h"
#include "device/bluetooth/dbus/bluetooth_adapter_client.h"
#include "device/bluetooth/dbus/bluetooth_agent_manager_client.h"
#include "device/bluetooth/dbus/bluetooth_battery_client.h"
#include "device/bluetooth/dbus/bluetooth_debug_manager_client.h"
#include "device/bluetooth/dbus/bluetooth_device_client.h"
#include "device/bluetooth/dbus/bluetooth_gatt_characteristic_client.h"
......@@ -115,6 +116,11 @@ bluez::BluezDBusManager::GetBluetoothDebugManagerClient() {
return client_bundle_->bluetooth_debug_manager_client();
}
BluetoothBatteryClient* bluez::BluezDBusManager::GetBluetoothBatteryClient() {
DCHECK(object_manager_support_known_);
return client_bundle_->bluetooth_battery_client();
}
BluetoothDeviceClient* bluez::BluezDBusManager::GetBluetoothDeviceClient() {
DCHECK(object_manager_support_known_);
return client_bundle_->bluetooth_device_client();
......
......@@ -26,6 +26,7 @@ namespace bluez {
// Style Note: Clients are sorted by names.
class BluetoothAdapterClient;
class BluetoothAgentManagerClient;
class BluetoothBatteryClient;
class BluetoothDebugManagerClient;
class BluetoothDeviceClient;
class BluetoothGattCharacteristicClient;
......@@ -119,6 +120,7 @@ class DEVICE_BLUETOOTH_EXPORT BluezDBusManager {
BluetoothAdapterClient* GetBluetoothAdapterClient();
BluetoothLEAdvertisingManagerClient* GetBluetoothLEAdvertisingManagerClient();
BluetoothAgentManagerClient* GetBluetoothAgentManagerClient();
BluetoothBatteryClient* GetBluetoothBatteryClient();
BluetoothDebugManagerClient* GetBluetoothDebugManagerClient();
BluetoothDeviceClient* GetBluetoothDeviceClient();
BluetoothGattCharacteristicClient* GetBluetoothGattCharacteristicClient();
......
// Copyright (c) 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "device/bluetooth/dbus/fake_bluetooth_battery_client.h"
#include "base/logging.h"
#include "device/bluetooth/dbus/fake_bluetooth_adapter_client.h"
#include "third_party/cros_system_api/dbus/service_constants.h"
namespace bluez {
FakeBluetoothBatteryClient::Properties::Properties(
const PropertyChangedCallback& callback)
: BluetoothBatteryClient::Properties(
nullptr,
bluetooth_battery::kBluetoothBatteryInterface,
callback) {}
FakeBluetoothBatteryClient::Properties::~Properties() = default;
void FakeBluetoothBatteryClient::Properties::Get(
dbus::PropertyBase* property,
dbus::PropertySet::GetCallback callback) {
DVLOG(1) << "Get " << property->name();
std::move(callback).Run(false);
}
void FakeBluetoothBatteryClient::Properties::GetAll() {
DVLOG(1) << "GetAll";
}
void FakeBluetoothBatteryClient::Properties::Set(
dbus::PropertyBase* property,
dbus::PropertySet::SetCallback callback) {
DVLOG(1) << "Set " << property->name();
std::move(callback).Run(false);
}
FakeBluetoothBatteryClient::FakeBluetoothBatteryClient() = default;
FakeBluetoothBatteryClient::~FakeBluetoothBatteryClient() = default;
void FakeBluetoothBatteryClient::Init(
dbus::Bus* bus,
const std::string& bluetooth_service_name) {}
void FakeBluetoothBatteryClient::AddObserver(Observer* observer) {
observers_.AddObserver(observer);
}
void FakeBluetoothBatteryClient::RemoveObserver(Observer* observer) {
observers_.RemoveObserver(observer);
}
std::vector<dbus::ObjectPath>
FakeBluetoothBatteryClient::GetBatteriesForAdapter(
const dbus::ObjectPath& adapter_path) {
if (adapter_path ==
dbus::ObjectPath(FakeBluetoothAdapterClient::kAdapterPath))
return battery_list_;
else
return std::vector<dbus::ObjectPath>();
}
FakeBluetoothBatteryClient::Properties*
FakeBluetoothBatteryClient::GetProperties(const dbus::ObjectPath& object_path) {
PropertiesMap::const_iterator iter = properties_map_.find(object_path);
if (iter != properties_map_.end())
return iter->second.get();
return nullptr;
}
} // namespace bluez
// Copyright (c) 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef DEVICE_BLUETOOTH_DBUS_FAKE_BLUETOOTH_BATTERY_CLIENT_H_
#define DEVICE_BLUETOOTH_DBUS_FAKE_BLUETOOTH_BATTERY_CLIENT_H_
#include <cstdint>
#include <map>
#include <memory>
#include <vector>
#include "base/observer_list.h"
#include "dbus/object_path.h"
#include "dbus/property.h"
#include "device/bluetooth/bluetooth_export.h"
#include "device/bluetooth/dbus/bluetooth_battery_client.h"
namespace bluez {
// FakeBluetoothBatteryClient simulates the behavior of the Bluetooth Daemon
// battery objects and is used both in test cases in place of a mock and on
// the Linux desktop.
class DEVICE_BLUETOOTH_EXPORT FakeBluetoothBatteryClient
: public BluetoothBatteryClient {
public:
struct Properties : public BluetoothBatteryClient::Properties {
explicit Properties(const PropertyChangedCallback& callback);
~Properties() override;
// dbus::PropertySet override
void Get(dbus::PropertyBase* property,
dbus::PropertySet::GetCallback callback) override;
void GetAll() override;
void Set(dbus::PropertyBase* property,
dbus::PropertySet::SetCallback callback) override;
};
FakeBluetoothBatteryClient();
~FakeBluetoothBatteryClient() override;
// BluetoothBatteryClient overrides
void Init(dbus::Bus* bus, const std::string& bluetooth_service_name) override;
void AddObserver(Observer* observer) override;
void RemoveObserver(Observer* observer) override;
std::vector<dbus::ObjectPath> GetBatteriesForAdapter(
const dbus::ObjectPath& adapter_path) override;
Properties* GetProperties(const dbus::ObjectPath& object_path) override;
private:
// List of observers interested in event notifications from us.
base::ObserverList<Observer>::Unchecked observers_;
using PropertiesMap =
std::map<const dbus::ObjectPath, std::unique_ptr<Properties>>;
PropertiesMap properties_map_;
std::vector<dbus::ObjectPath> battery_list_;
};
} // namespace bluez
#endif // DEVICE_BLUETOOTH_DBUS_FAKE_BLUETOOTH_DEVICE_CLIENT_H_
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment