fix(whatsminer): correct inverted is_mining logic across all backends#187
Merged
b-rowan merged 5 commits into256foundation:masterfrom Mar 19, 2026
Merged
Conversation
Member
From V2 miner. V1 should be the same. |
Member
Another firmware version. |
Member
Older still. This might be closer to the V1 API. |
Member
Here's the result from a V3 device. |
V1/V2: The `btmineroff`/`mineroff` field means "miner is off", so "true" means mining is OFF. The old code checked `l != "false"` which returned true (mining) when the miner was actually paused. Also added a fallback extractor at `/Msg/mineroff` for newer firmware responding to the legacy API. V3: Had no implementation (always returned true). Now extracts the `working` field from `get.device.info` which directly indicates whether the miner is active. Verified against M60SVK40 hardware: pause() correctly flips is_mining to false, resume() flips it back to true. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
7814f51 to
9ac2e17
Compare
Update V1 status.json fixture to use "mineroff" at /Msg/mineroff matching real firmware responses (confirmed across fw versions 20230208 through 20250214). Add is_mining assertion to V1 integration test. Add V2 parse_is_mining unit tests. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add full data parser integration tests for V2 and V3 backends with realistic fixture data. V2 fixtures based on fw 20230803 sample data from PR review. V3 fixtures based on real M60SVK40 API responses. Both tests exercise the complete collect → parse pipeline and assert on is_mining along with hashrate, wattage, pools, fans, and other parsed fields. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Member
|
I'll see if I can confirm V1 results tomorrow, we have some M20s laying around that use 1.4.0 |
…ct modules Move integration tests (full pipeline with MockAPIClient) into `integration_tests` modules, keeping unit tests (direct parser calls) in `tests` modules. No logic changes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
a9e63e2 to
ae58cb0
Compare
Contributor
Author
|
I left the /SUMMARY/0/btmineroff extraction path in assuming it was for V1 |
Member
That's my guess too, but it might be one or the other for V1/V2, I'll know more tomorrow. |
Member
V1 is in fact the one with |
b-rowan
requested changes
Mar 19, 2026
- V1: use /Msg/btmineroff (confirmed by real V1 hardware, fw 20220919) - V2: use /Msg/mineroff only (remove btmineroff fallback) - Revert parse_is_mining to extract_map one-liner style - Remove redundant unit tests covered by integration tests - Update V1 status.json fixture with real V1 response data Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
b-rowan
approved these changes
Mar 19, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
parse_is_miningwas inverted — thebtmineroff/minerofffield means "miner is off", so"true"means mining is OFF, but the old code returnedtrue(mining active). Added fallback extraction at/Msg/minerofffor newer firmware responding to the legacy API.true). Now extracts theworkingfield fromget.device.infoat/msg/miner/working.miner_off,working) to match the actual API field names so the inversion logic is self-documenting.Test plan
parse_is_miningcoveringmineroff="true",mineroff="false", and missing fieldpause()→is_mining=false,resume()→is_mining=truepause()→is_mining=false,resume()→is_mining=true🤖 Generated with Claude Code