Skip to content

Commit

Permalink
Eliminate use of globals to generate ids (#371)
Browse files Browse the repository at this point in the history
  • Loading branch information
bdraco authored Jul 17, 2024
1 parent b217e66 commit 2e223d7
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 52 deletions.
3 changes: 1 addition & 2 deletions aiohomekit/controller/ble/pairing.py
Original file line number Diff line number Diff line change
Expand Up @@ -830,8 +830,7 @@ async def _read_signature(

async def _async_fetch_gatt_database(self) -> Accessories:
logger.debug("%s: Fetching GATT database; rssi=%s", self.name, self.rssi)
accessory = Accessory()
accessory.aid = 1
accessory = Accessory(1)
# Never use the cache when fetching the GATT database
services_to_link: list[tuple[Service, list[int]]] = []
services = self.client.services
Expand Down
11 changes: 5 additions & 6 deletions aiohomekit/model/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
CharacteristicsTypes,
)
from .feature_flags import FeatureFlags
from .mixin import get_id
from .services import Service, ServicesTypes

__all__ = [
Expand Down Expand Up @@ -178,9 +177,9 @@ def iid(self, iid: int) -> Characteristic | None:
class Accessory:
"""Represents a HomeKit accessory."""

def __init__(self) -> None:
def __init__(self, aid: int) -> None:
"""Initialize a new accessory."""
self.aid = get_id()
self.aid = aid
self._next_id = 0
self.services = Services()
self.characteristics = Characteristics(self.services)
Expand All @@ -189,6 +188,7 @@ def __init__(self) -> None:
@classmethod
def create_with_info(
cls,
aid: int,
name: str,
manufacturer: str,
model: str,
Expand All @@ -200,7 +200,7 @@ def create_with_info(
This method should only be used for testing purposes as it assigns
the next available ids to the accessory and services.
"""
self = cls()
self = cls(aid)

accessory_info = self.add_service(ServicesTypes.ACCESSORY_INFORMATION)
accessory_info.add_char(CharacteristicsTypes.IDENTIFY, description="Identify")
Expand Down Expand Up @@ -278,8 +278,7 @@ def needs_polling(self) -> bool:
@classmethod
def create_from_dict(cls, data: dict[str, Any]) -> Accessory:
"""Create an accessory from a dict."""
accessory = cls()
accessory.aid = data["aid"]
accessory = cls(data["aid"])

for service_data in data["services"]:
service = accessory.add_service(
Expand Down
23 changes: 0 additions & 23 deletions aiohomekit/model/mixin.py

This file was deleted.

32 changes: 21 additions & 11 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

from aiohomekit import Controller
from aiohomekit.controller.ip import IpPairing
from aiohomekit.model import Accessory, mixin as model_mixin
from aiohomekit.model import Accessory
from aiohomekit.model.characteristics import CharacteristicsTypes
from aiohomekit.model.services import ServicesTypes

Expand Down Expand Up @@ -82,7 +82,9 @@ def mock_asynczeroconf():


@pytest.fixture
async def controller_and_unpaired_accessory(request, mock_asynczeroconf, event_loop):
async def controller_and_unpaired_accessory(
request, mock_asynczeroconf, event_loop, id_factory
):
available_port = next_available_port()

config_file = tempfile.NamedTemporaryFile(delete=False)
Expand All @@ -104,12 +106,9 @@ async def controller_and_unpaired_accessory(request, mock_asynczeroconf, event_l
)
config_file.close()

# Make sure get_id() numbers are stable between tests
model_mixin.id_counter = 0

httpd = AccessoryServer(config_file.name, None)
accessory = Accessory.create_with_info(
"Testlicht", "lusiardi.de", "Demoserver", "0001", "0.1"
id_factory(), "Testlicht", "lusiardi.de", "Demoserver", "0001", "0.1"
)
lightBulbService = accessory.add_service(ServicesTypes.LIGHTBULB)
lightBulbService.add_char(CharacteristicsTypes.ON, value=False)
Expand Down Expand Up @@ -138,7 +137,9 @@ def _shutdown():


@pytest.fixture
async def controller_and_paired_accessory(request, event_loop, mock_asynczeroconf):
async def controller_and_paired_accessory(
request, event_loop, mock_asynczeroconf, id_factory
):
available_port = next_available_port()

config_file = tempfile.NamedTemporaryFile(delete=False)
Expand Down Expand Up @@ -166,12 +167,9 @@ async def controller_and_paired_accessory(request, event_loop, mock_asynczerocon
config_file.write(data)
config_file.close()

# Make sure get_id() numbers are stable between tests
model_mixin.id_counter = 0

httpd = AccessoryServer(config_file.name, None)
accessory = Accessory.create_with_info(
"Testlicht", "lusiardi.de", "Demoserver", "0001", "0.1"
id_factory(), "Testlicht", "lusiardi.de", "Demoserver", "0001", "0.1"
)
lightBulbService = accessory.add_service(ServicesTypes.LIGHTBULB)
lightBulbService.add_char(CharacteristicsTypes.ON, value=False)
Expand Down Expand Up @@ -253,3 +251,15 @@ async def pairings(request, controller_and_paired_accessory, event_loop):
@pytest.fixture(autouse=True)
def configure_test_logging(caplog):
caplog.set_level(logging.DEBUG)


@pytest.fixture()
def id_factory():
id_counter = 0

def _get_id():
nonlocal id_counter
id_counter += 1
return id_counter

yield _get_id
6 changes: 4 additions & 2 deletions tests/test_testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,10 @@ async def test_update_named_service_events():
assert accessories.aid(1).characteristics.iid(8).value == 1


async def test_update_named_service_events_manual_accessory():
async def test_update_named_service_events_manual_accessory(id_factory):
accessories = Accessories()
accessory = Accessory.create_with_info(
id_factory(),
name="TestLight",
manufacturer="Test Mfr",
model="Test Bulb",
Expand All @@ -117,9 +118,10 @@ async def test_update_named_service_events_manual_accessory():
]


async def test_update_named_service_events_manual_accessory_auto_requires():
async def test_update_named_service_events_manual_accessory_auto_requires(id_factory):
accessories = Accessories()
accessory = Accessory.create_with_info(
id_factory(),
name="TestLight",
manufacturer="Test Mfr",
model="Test Bulb",
Expand Down
16 changes: 8 additions & 8 deletions tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ def test_shorten_invalid_uuid():
shorten_uuid("NOT_A_VALID_UUID")


def test_clamp_enum_valid_vals():
a = Accessory()
def test_clamp_enum_valid_vals(id_factory):
a = Accessory(id_factory())
tv_service = a.add_service(service_type=ServicesTypes.TELEVISION)
char = tv_service.add_char(
CharacteristicsTypes.REMOTE_KEY,
Expand All @@ -86,8 +86,8 @@ def test_clamp_enum_valid_vals():
assert valid_vals == {RemoteKeyValues.PLAY_PAUSE}


def test_clamp_enum_min_max():
a = Accessory()
def test_clamp_enum_min_max(id_factory):
a = Accessory(id_factory())
tv_service = a.add_service(service_type=ServicesTypes.TELEVISION)
char = tv_service.add_char(
CharacteristicsTypes.REMOTE_KEY,
Expand All @@ -101,8 +101,8 @@ def test_clamp_enum_min_max():
assert valid_vals == {RemoteKeyValues.PLAY_PAUSE}


def test_clamp_enum_min_max_single_press():
a = Accessory()
def test_clamp_enum_min_max_single_press(id_factory):
a = Accessory(id_factory())
tv_service = a.add_service(service_type=ServicesTypes.STATELESS_PROGRAMMABLE_SWITCH)
char = tv_service.add_char(
CharacteristicsTypes.INPUT_EVENT,
Expand All @@ -116,8 +116,8 @@ def test_clamp_enum_min_max_single_press():
assert valid_vals == {InputEventValues.SINGLE_PRESS}


def test_clamp_enum_min_max_unclamped_button_press():
a = Accessory()
def test_clamp_enum_min_max_unclamped_button_press(id_factory):
a = Accessory(id_factory())
tv_service = a.add_service(service_type=ServicesTypes.STATELESS_PROGRAMMABLE_SWITCH)
char = tv_service.add_char(
CharacteristicsTypes.INPUT_EVENT,
Expand Down

0 comments on commit 2e223d7

Please sign in to comment.