-
Notifications
You must be signed in to change notification settings - Fork 86
Add set_profile method to LuxOS
#409
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
Open
wilfredallyn
wants to merge
5
commits into
UpstreamData:master
Choose a base branch
from
wilfredallyn:feat/set-preset
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+232
−8
Open
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
65c78ea
feat: add set_preset() and refactor ATM toggle into _switch_profile()
wilfredallyn cbfa008
feat: add set_preset() stub and tests (RED)
wilfredallyn 3c9cf37
feat: implement set_preset() for LuxOS backend (GREEN)
wilfredallyn f1271b6
feat: rename set_preset() to set_profile() with fuzzy matching
wilfredallyn 4e9fa59
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -177,6 +177,90 @@ async def atm_enabled(self) -> bool | None: | |
| pass | ||
| return None | ||
|
|
||
| async def _switch_profile(self, profile_name: str) -> dict: | ||
| """Switch LuxOS profile with ATM temporarily disabled if needed. | ||
|
|
||
| Handles the ATM disable/re-enable dance required by LuxOS when | ||
| switching profiles. ATM is always re-enabled if it was on, even | ||
| if the profile switch fails. | ||
|
|
||
| Parameters: | ||
| profile_name: The name of the profile to switch to. | ||
|
|
||
| Returns: | ||
| The raw response dict from profileset. | ||
|
|
||
| Raises: | ||
| APIError: If the RPC call returns an API-level error. | ||
| Exception: If the profile switch fails for other reasons. | ||
| """ | ||
| re_enable_atm = False | ||
| try: | ||
| if await self.atm_enabled(): | ||
| re_enable_atm = True | ||
| await self.rpc.atmset(enabled=False) | ||
| return await self.rpc.profileset(profile_name) | ||
| finally: | ||
| if re_enable_atm: | ||
| try: | ||
| await self.rpc.atmset(enabled=True) | ||
| except Exception as e: | ||
| logging.warning( | ||
| f"{self} - Failed to re-enable ATM after profile switch: {e}" | ||
| ) | ||
|
|
||
| @staticmethod | ||
| def _match_profile_name(name: str, profile_names: list[str]) -> str | None: | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When calling |
||
| """Match user input to an available profile name. | ||
|
|
||
| Tries exact match first, then case-insensitive, then checks if | ||
| appending "MHz" yields a match (so "190" matches "190MHz"). | ||
| """ | ||
| if name in profile_names: | ||
| return name | ||
|
|
||
| lower = name.lower() | ||
| for p in profile_names: | ||
| if p.lower() == lower: | ||
| return p | ||
|
|
||
| for p in profile_names: | ||
| if p.lower() == lower + "mhz": | ||
| return p | ||
|
|
||
| return None | ||
|
|
||
| async def set_profile(self, name: str) -> bool: | ||
| config = await self.get_config() | ||
|
|
||
| # Validate profile name exists | ||
| if not hasattr(config.mining_mode, "available_presets"): | ||
| logging.warning(f"{self} - Mining mode does not support profiles") | ||
| return False | ||
|
|
||
| available_presets = getattr(config.mining_mode, "available_presets", []) | ||
| profile_names = [p.name for p in available_presets if p.name is not None] | ||
|
|
||
| matched_name = self._match_profile_name(name, profile_names) | ||
| if matched_name is None: | ||
| logging.warning( | ||
| f"{self} - Profile '{name}' not found in available profiles: {profile_names}" | ||
| ) | ||
| return False | ||
|
|
||
| try: | ||
| result = await self._switch_profile(matched_name) | ||
| except APIError: | ||
| raise | ||
| except Exception as e: | ||
| logging.warning(f"{self} - Failed to set profile: {e}") | ||
| return False | ||
|
|
||
| if result["PROFILE"][0]["Profile"] == matched_name: | ||
| return True | ||
| else: | ||
| return False | ||
|
|
||
| async def set_power_limit(self, wattage: int) -> bool: | ||
| config = await self.get_config() | ||
|
|
||
|
|
@@ -201,17 +285,10 @@ async def set_power_limit(self, wattage: int) -> bool: | |
| return False | ||
|
|
||
| # Set power to highest preset <= wattage | ||
| # If ATM enabled, must disable it before setting power limit | ||
| new_preset = max(valid_presets, key=lambda x: valid_presets[x]) | ||
|
|
||
| re_enable_atm = False | ||
| try: | ||
| if await self.atm_enabled(): | ||
| re_enable_atm = True | ||
| await self.rpc.atmset(enabled=False) | ||
| result = await self.rpc.profileset(new_preset) | ||
| if re_enable_atm: | ||
| await self.rpc.atmset(enabled=True) | ||
| result = await self._switch_profile(new_preset) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated |
||
| except APIError: | ||
| raise | ||
| except Exception as e: | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Empty file.
136 changes: 136 additions & 0 deletions
136
tests/miners_tests/backends_tests/luxminer_tests/test_set_preset.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,136 @@ | ||
| """Tests for LuxOS set_profile() method.""" | ||
|
|
||
| import logging | ||
| import unittest | ||
| from unittest.mock import AsyncMock, MagicMock, patch | ||
|
|
||
| from pyasic.config.mining import MiningModeConfig | ||
| from pyasic.config.mining.presets import MiningPreset | ||
|
|
||
|
|
||
| def _make_preset(name, power=1000, frequency=400, voltage=8.9): | ||
| """Helper to create a MiningPreset.""" | ||
| return MiningPreset( | ||
| name=name, | ||
| power=power, | ||
| hashrate=80.0, | ||
| tuned=True, | ||
| modded_psu=False, | ||
| frequency=frequency, | ||
| voltage=voltage, | ||
| ) | ||
|
|
||
|
|
||
| def _make_config( | ||
| active_preset_name="415MHz", available_preset_names=("190MHz", "415MHz", "565MHz") | ||
| ): | ||
| """Helper to create a mock config with presets.""" | ||
| presets = [_make_preset(n) for n in available_preset_names] | ||
| active = next((p for p in presets if p.name == active_preset_name), presets[0]) | ||
| config = MagicMock() | ||
| config.mining_mode.available_presets = presets | ||
| config.mining_mode.active_preset = active | ||
| return config | ||
|
|
||
|
|
||
| class TestSetProfile(unittest.IsolatedAsyncioTestCase): | ||
| """Test LuxOS set_profile() behavior.""" | ||
|
|
||
| def _make_miner(self, atm_enabled=True, config=None, profileset_result=None): | ||
| """Create a mock LuxOS miner with controllable behavior.""" | ||
| from pyasic.miners.backends.luxminer import LUXMiner | ||
|
|
||
| miner = LUXMiner.__new__(LUXMiner) | ||
| miner.rpc = MagicMock() | ||
| miner.rpc.atmset = AsyncMock() | ||
| miner.rpc.profileset = AsyncMock( | ||
| return_value=profileset_result or {"PROFILE": [{"Profile": "415MHz"}]} | ||
| ) | ||
| miner.atm_enabled = AsyncMock(return_value=atm_enabled) | ||
| miner.get_config = AsyncMock(return_value=config or _make_config()) | ||
| miner.ip = "192.168.1.237" | ||
| return miner | ||
|
|
||
| async def test_switches_preset_with_atm_on(self): | ||
| """When ATM is on, should disable ATM, switch, then re-enable ATM.""" | ||
| miner = self._make_miner( | ||
| atm_enabled=True, | ||
| profileset_result={"PROFILE": [{"Profile": "190MHz"}]}, | ||
| ) | ||
|
|
||
| result = await miner.set_profile("190MHz") | ||
|
|
||
| self.assertTrue(result) | ||
| miner.rpc.atmset.assert_any_call(enabled=False) | ||
| miner.rpc.profileset.assert_called_once_with("190MHz") | ||
| miner.rpc.atmset.assert_any_call(enabled=True) | ||
|
|
||
| async def test_switches_preset_with_atm_off(self): | ||
| """When ATM is off, should switch without toggling ATM.""" | ||
| miner = self._make_miner( | ||
| atm_enabled=False, | ||
| profileset_result={"PROFILE": [{"Profile": "190MHz"}]}, | ||
| ) | ||
|
|
||
| result = await miner.set_profile("190MHz") | ||
|
|
||
| self.assertTrue(result) | ||
| miner.rpc.atmset.assert_not_called() | ||
| miner.rpc.profileset.assert_called_once_with("190MHz") | ||
|
|
||
| async def test_invalid_preset_name_returns_false(self): | ||
| """Should return False when preset name doesn't exist.""" | ||
| miner = self._make_miner() | ||
|
|
||
| result = await miner.set_profile("nonexistent_profile") | ||
|
|
||
| self.assertFalse(result) | ||
| miner.rpc.profileset.assert_not_called() | ||
|
|
||
| async def test_atm_re_enable_failure_returns_true_and_warns(self): | ||
| """If preset switches but ATM re-enable fails, return True and log warning.""" | ||
| miner = self._make_miner( | ||
| atm_enabled=True, | ||
| profileset_result={"PROFILE": [{"Profile": "190MHz"}]}, | ||
| ) | ||
| # First call (disable) succeeds, second call (re-enable) fails | ||
| miner.rpc.atmset.side_effect = [None, Exception("ATM re-enable failed")] | ||
|
|
||
| with self.assertLogs(level=logging.WARNING): | ||
| result = await miner.set_profile("190MHz") | ||
|
|
||
| self.assertTrue(result) | ||
| miner.rpc.profileset.assert_called_once_with("190MHz") | ||
|
|
||
| async def test_fuzzy_match_case_insensitive(self): | ||
| """Should match preset name case-insensitively.""" | ||
| miner = self._make_miner( | ||
| atm_enabled=False, | ||
| profileset_result={"PROFILE": [{"Profile": "190MHz"}]}, | ||
| ) | ||
|
|
||
| result = await miner.set_profile("190mhz") | ||
|
|
||
| self.assertTrue(result) | ||
| miner.rpc.profileset.assert_called_once_with("190MHz") | ||
|
|
||
| async def test_fuzzy_match_number_only(self): | ||
| """Should match '190' to '190MHz'.""" | ||
| miner = self._make_miner( | ||
| atm_enabled=False, | ||
| profileset_result={"PROFILE": [{"Profile": "190MHz"}]}, | ||
| ) | ||
|
|
||
| result = await miner.set_profile("190") | ||
|
|
||
| self.assertTrue(result) | ||
| miner.rpc.profileset.assert_called_once_with("190MHz") | ||
|
|
||
| async def test_profileset_failure_returns_false(self): | ||
| """If RPC profileset fails, should return False.""" | ||
| miner = self._make_miner(atm_enabled=False) | ||
| miner.rpc.profileset.side_effect = Exception("RPC error") | ||
|
|
||
| result = await miner.set_profile("190MHz") | ||
|
|
||
| self.assertFalse(result) |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created helper method that disables and re-enables ATM so don’t duplicate logic in methods that need to do this