feat(throttle): native manual throttle (SetThrottle + throttle_percent)#289
Conversation
|
Trying to understand what exactly this does... Seems like it is essentially a scalar on top of the preset tuning target? Can you help me understand why this is needed? My preference would be that a use sets a new target power value rather than this, especially since that is already standardized across firmwares... We already have the concept of a |
|
Good questions — happy to clarify. You're right that it's a scalar on top of the active preset. The key point is that on VNish it's a different operation from setting a power target: I think you're spot on that the resulting (throttled-back) power belongs in The real open question is where the setter should live. Options I see:
I don't have a strong attachment to the trait shape; I'd rather fit it to your design. Which direction would you prefer? Happy to rework the PR accordingly. |
|
Tough part for me is that this doesn't fit cleanly into Curious what the rest of the team thinks, maybe they have better ideas. |
|
Agreed — For the set side I'd rather follow whatever shape you land on for tuning targets (this overlaps with the preset discussion in #291) instead of inventing a parallel path. And since WhatsMiners share the concept, it seems worth settling on one abstraction that covers both — glad to do the VNish part once the team has a direction. No rush on my end; I can hold this PR open until that's settled. |
|
@pos-ei-don have you tried the ePIC firmware? It has power tune, and it will guarantee the power within 30sec of changing it. also a lower dev fee |
|
I'll do more talking with the team, but tempted to decline this for now based on a couple things:
|
|
Totally fair — no objection to declining it here. For context though, since I rely on this daily: the throttle isn't a replacement for power-target tuning, it's a fast, linear, external-signal power lever, and a couple of things it does that tuning doesn't:
On the "firmware should handle it" point — VNish exposes this as a first-class, promoted firmware feature (the manual throttle endpoint), available in current firmware, so it's stable and not going away. All that said, I agree it adds complexity and that presets (#291) + power targets cover the common cases well — so I'm happy to keep the manual throttle as a fork-local extra and not push it upstream. I'll keep using it either way. Thanks for taking it to the team. |
|
Thanks @cfilipescu — I haven't tried ePIC, and hadn't heard much beyond the name, but a guaranteed power target within 30s (and a lower dev fee) sounds genuinely interesting. Do you run it yourself? I'd love to hear more about how the power-tune behaves in practice — particularly how tightly and quickly it tracks a changing target, since fast power-following is exactly what I'm after. |
|
Ok, I think we have a workable solution to this now, I think I want to change this to |
Full disclosure, I work for the company. This was a new feature in the latest firmware release, and it only works on S21 miners because the PSU has a power reading. We confirmed on multiple systems, and the power is within 50W of the set target. If you want more information, I can share my Telegram handle. |
|
Two things: The manual throttle endpoint is now officially documented in the VNish 1.3.4 OpenAPI — I added that schema as On the rename: agreed — |
|
Thanks — yes, I'd love your Telegram handle. This is genuinely interesting to me. The first thing I'd want to figure out is whether I can even try it on my hardware: I run an S19 Pro Hydro (water-cooled). I see PowerPlay is open source — And did I understand correctly that the new power-target feature is S21-only for now because it relies on the PSU exposing a power reading? If the blocker is purely that PSU power telemetry, could it be ported to S19-class hardware where the PSU exposes it? Since the firmware is open source, that might even be something I'd be happy to help contribute. |
|
Renamed as requested: On stacking: |
Sadly we don't support hydro systems right now, but if I can get remote access to a system, I can add it. Yes that is correct if you put a PSU that supports power it would work. At this time the FW code is not open sourced. my telegram handle is @cfilipescu |
Having seen #297, and that it doesn't exist in 1.2.x, can you move the implementation for this version to a new |
|
Done — pushed the split in
One thing I want to flag before we settle on this, though. What makes me hesitate specifically here: the capability query is instance-level ( fn supports_set_tuning_percent(&self) -> bool { self.version >= Version::new(1, 3, 0) }— gives every consumer exactly the right answer too, in ~15 lines instead of a full backend copy. That seems to satisfy "accurate to what it supports" without the duplication. The full split clearly wins once a version genuinely diverges (like braiins did across GraphQL schemas), but vnish isn't there yet. I've pushed the split since that's what you asked for and it's ready to merge — but do you actually want the per-version backend copy here, or would you prefer the instance-level gate while the delta is this small? Happy either way. Side note: the |
|
Looks good, needs merge conflicts fixed, and may need updates for #293 (or in it), but good to go. |
0bec055 to
3b44970
Compare
… split - SetTuningPercent/GetTuningPercent trait, MinerData.tuning_percent, DataField::TuningPercent - VNish 1.3.x manual throttle via POST /mining/throttle (clamped 20..=100) - split vnish backend into v1_2_0 / v1_3_0; tuning_percent gated to v1_3_0 (fw >= 1.3.0), v1_2_0 stays on trait defaults (1.2.x has no throttle endpoint) - constructor routes on detected firmware version (braiins multi-version pattern)
3b44970 to
31a206b
Compare
|
Rebased onto current master (v0.7.1) and resolved the conflicts. Also did the #293 follow-up you flagged: since CI green (Cargo + Python). |
First off — thank you for 0.7.0 with native VNish tuning. That's exactly the foundation I was hoping for: it lets me drop my REST shims and source the control from asic-rs the way you laid out in your review.
Forward-porting my 0.6.x additions onto your 0.7 line was already a good chunk of work on my side — but I think it's the more important investment, so that's where I'm putting the effort: contributing what I need back here as proper library code instead of carrying patches. Starting with this one — native throttle. Native named-preset listing and the configured thermal-limit fields come next, and I'm happy to shape them to fit your model (the preset side in particular has a design question I'd value your input on).
On the Home Assistant integration itself, let me be straight: it now does everything I need — I've reached the goals I had for it, so I'm not putting much more into it. Splitting the existing GUI into a clean series of separate upstream PRs would be a lot of work, and since it already does what I need, that's effort I'm not planning to put in — so I'm keeping it as a runnable alpha in my fork rather than opening integration PRs. Anyone who wants it can just install it, and you're very welcome to take any part of it that's useful.
Really appreciate the work you've put into asic-rs.
What this PR adds
SetThrottlecontrol trait (set_throttle(percent)/supports_set_throttle, defaultfalse), wired intoHasMinerControlnext toSetPowerLimit.GetThrottle+MinerData.throttle_percent(100 = unthrottled), wired intoGetMinerData; newDataField::Throttle./summaryminer_status/throttled; writesPOST /mining/throttle(clamped 20..=100).Miner.set_throttle/get_throttle/supports_set_throttle,MinerData.throttle_percent) +.pyi.Tested against an S19 Pro Hydro (VNish): read path verified live; CI green (
cargo test --all+ Python tests).