Skip to content

Commit b20712a

Browse files
author
Yu Leng (from Dev Box)
committed
PowerDisplay: Drop VcpProbeDisposition and the duplicated probe-attempt log
VcpProbeDisposition was a three-state enum carrying one bit anything branched on. Only PhysicalMonitorUnavailable drove a decision -- the abort in ProbeAsync -- while Success and Indeterminate existed to be mapped back to strings for a single log line. The bit was already computable from LastError through the same classifier the derived property called, so it is now a plain IsPhysicalMonitorUnavailable on the observation and the enum is gone. The status word in Complete() reads off IsSuccess and that property directly. Every emitted string is unchanged: IsSuccess true with a handle-class LastError is unreachable, because handle-class codes are not transient, so they break out of the attempt loop and fall through to the terminal Indeterminate, whose value is VcpFeatureValue.Invalid. The two per-attempt LogDebug templates in the success arm differed in one token after the early-return change made both arms terminal. They collapse into one call with the status word interpolated, and the two Complete() calls into one over a conditional observation. Tests keep the same coverage: the six Disposition assertions become IsSuccess / IsPhysicalMonitorUnavailable pairs, except the one in ProbeAsync_TransientFailureThenSuccessRetriesWithPacing, which restated the IsSuccess assertion directly above it and is dropped.
1 parent ca3cd3c commit b20712a

2 files changed

Lines changed: 29 additions & 43 deletions

File tree

src/modules/powerdisplay/PowerDisplay.Lib.UnitTests/VcpFeatureProbeServiceTests.cs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ public async Task ProbeAsync_TransientFailureThenSuccessRetriesWithPacing()
4646

4747
Assert.AreEqual(2, reader.CallCount);
4848
Assert.IsTrue(result[0x10].IsSuccess);
49-
Assert.AreEqual(VcpProbeDisposition.Success, result[0x10].Disposition);
5049
Assert.AreEqual(2, result[0x10].Attempts);
5150
Assert.AreEqual(DdcErrorClassifier.ErrorGraphicsDdcCiInvalidMessageCommand, result[0x10].LastError);
5251
CollectionAssert.AreEqual(
@@ -99,7 +98,7 @@ public async Task ProbeAsync_PhysicalMonitorUnavailableStopsRemainingFeatureProb
9998
Assert.AreEqual(1, reader.CallCount);
10099
CollectionAssert.AreEqual(new byte[] { 0x10 }, reader.Codes);
101100
Assert.AreEqual(1, result.Count);
102-
Assert.AreEqual(VcpProbeDisposition.PhysicalMonitorUnavailable, result[0x10].Disposition);
101+
Assert.IsTrue(result[0x10].IsPhysicalMonitorUnavailable);
103102
Assert.AreEqual(errorCode, result[0x10].LastError);
104103
}
105104

@@ -115,8 +114,9 @@ public async Task ProbeAsync_VcpNotSupportedContinuesWithRemainingFeatures()
115114

116115
Assert.AreEqual(2, reader.CallCount);
117116
CollectionAssert.AreEqual(new byte[] { 0x10, 0x12 }, reader.Codes);
118-
Assert.AreEqual(VcpProbeDisposition.Indeterminate, result[0x10].Disposition);
119-
Assert.AreEqual(VcpProbeDisposition.Success, result[0x12].Disposition);
117+
Assert.IsFalse(result[0x10].IsSuccess);
118+
Assert.IsFalse(result[0x10].IsPhysicalMonitorUnavailable);
119+
Assert.IsTrue(result[0x12].IsSuccess);
120120
}
121121

122122
[TestMethod]
@@ -153,7 +153,8 @@ public async Task ProbeAsync_TransientFailureThenInvalidRangeKeepsLastError()
153153
Assert.AreEqual(2, reader.CallCount);
154154
Assert.AreEqual(2, result[0x10].Attempts);
155155
Assert.IsTrue(result[0x10].Replied);
156-
Assert.AreEqual(VcpProbeDisposition.Indeterminate, result[0x10].Disposition);
156+
Assert.IsFalse(result[0x10].IsSuccess);
157+
Assert.IsFalse(result[0x10].IsPhysicalMonitorUnavailable);
157158

158159
// Returning early on the reply must not discard the error the earlier attempt reported.
159160
Assert.AreEqual(DdcErrorClassifier.ErrorGraphicsDdcCiInvalidMessageChecksum, result[0x10].LastError);
@@ -169,7 +170,8 @@ public async Task ProbeAsync_ThrowingReadIsContainedToItsOwnCode()
169170

170171
var result = await service.ProbeAsync(new IntPtr(1), CancellationToken.None);
171172

172-
Assert.AreEqual(VcpProbeDisposition.Indeterminate, result[0x10].Disposition);
173+
Assert.IsFalse(result[0x10].IsSuccess);
174+
Assert.IsFalse(result[0x10].IsPhysicalMonitorUnavailable);
173175
Assert.IsFalse(result[0x10].Replied);
174176
Assert.IsTrue(result[0x12].IsSuccess);
175177
Assert.AreEqual(40, result[0x12].Value.Current);

src/modules/powerdisplay/PowerDisplay.Lib/Drivers/DDC/VcpFeatureProbeService.cs

Lines changed: 21 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public async Task<IReadOnlyDictionary<byte, VcpProbeObservation>> ProbeAsync(
4444

4545
// These errors invalidate the physical-monitor handle, not just the current
4646
// VCP feature. Avoid issuing more I2C requests against a stale handle.
47-
if (observation.Disposition == VcpProbeDisposition.PhysicalMonitorUnavailable)
47+
if (observation.IsPhysicalMonitorUnavailable)
4848
{
4949
break;
5050
}
@@ -95,28 +95,19 @@ ex is not OperationCanceledException &&
9595
// DDCCI_VCP_NOT_SUPPORTED instead, so a reply proves support even when the
9696
// reported range cannot scale a percentage.
9797
var value = new VcpFeatureValue((int)result.Current, 0, (int)result.Maximum);
98-
if (value.IsValid)
99-
{
100-
Logger.LogDebug(
101-
$"DDC: [max-compat] VCP probe attempt " +
102-
$"(handle=0x{handle:X}, code=0x{code:X2}, attempt={attempt}/{MaxAttempts}, " +
103-
$"status=success, current={result.Current}, maximum={result.Maximum})");
104-
return Complete(
105-
handle,
106-
VcpProbeObservation.Success(code, value, attempt, lastError));
107-
}
98+
var observation = value.IsValid
99+
? VcpProbeObservation.Success(code, value, attempt, lastError)
100+
: VcpProbeObservation.Indeterminate(code, lastError, attempt, replied: true);
108101

109102
Logger.LogDebug(
110103
$"DDC: [max-compat] VCP probe attempt " +
111104
$"(handle=0x{handle:X}, code=0x{code:X2}, attempt={attempt}/{MaxAttempts}, " +
112-
$"status=invalid-range, current={result.Current}, maximum={result.Maximum})");
113-
114-
// A degenerate range is still a reply, and the reply is the only thing the
115-
// caller reads off this probe. Another attempt could not change that, so the
116-
// remaining budget is not spent on the I2C bus.
117-
return Complete(
118-
handle,
119-
VcpProbeObservation.Indeterminate(code, lastError, attempt, replied: true));
105+
$"status={(value.IsValid ? "success" : "invalid-range")}, " +
106+
$"current={result.Current}, maximum={result.Maximum})");
107+
108+
// A reply settles the only question this probe asks, so the remaining budget is
109+
// not spent on the I2C bus even when the reported range is degenerate.
110+
return Complete(handle, observation);
120111
}
121112
else
122113
{
@@ -142,12 +133,11 @@ ex is not OperationCanceledException &&
142133

143134
private static VcpProbeObservation Complete(IntPtr handle, VcpProbeObservation observation)
144135
{
145-
var status = observation.Disposition switch
146-
{
147-
VcpProbeDisposition.Success => "success",
148-
VcpProbeDisposition.PhysicalMonitorUnavailable => "physical-monitor-unavailable",
149-
_ => "indeterminate",
150-
};
136+
var status = observation.IsSuccess
137+
? "success"
138+
: observation.IsPhysicalMonitorUnavailable
139+
? "physical-monitor-unavailable"
140+
: "indeterminate";
151141
var message =
152142
$"DDC: [max-compat] VCP probe outcome " +
153143
$"(handle=0x{handle:X}, code=0x{observation.Code:X2}, attempts={observation.Attempts}, " +
@@ -169,13 +159,6 @@ private static string FormatError(int? errorCode) =>
169159
errorCode.HasValue ? $"0x{unchecked((uint)errorCode.Value):X8}" : "none";
170160
}
171161

172-
internal enum VcpProbeDisposition
173-
{
174-
Success,
175-
Indeterminate,
176-
PhysicalMonitorUnavailable,
177-
}
178-
179162
internal readonly record struct VcpReadAttempt(bool IsSuccess, uint Current, uint Maximum, int ErrorCode)
180163
{
181164
public static VcpReadAttempt Success(uint current, uint maximum) => new(true, current, maximum, 0);
@@ -197,11 +180,12 @@ internal readonly record struct VcpProbeObservation(
197180
{
198181
public bool IsSuccess => Value.IsValid;
199182

200-
public VcpProbeDisposition Disposition => IsSuccess
201-
? VcpProbeDisposition.Success
202-
: LastError is int errorCode && DdcErrorClassifier.IsPhysicalMonitorUnavailable(errorCode)
203-
? VcpProbeDisposition.PhysicalMonitorUnavailable
204-
: VcpProbeDisposition.Indeterminate;
183+
/// <summary>
184+
/// Gets a value indicating whether the failure invalidated the physical-monitor handle
185+
/// itself, so no further request may be issued against it.
186+
/// </summary>
187+
public bool IsPhysicalMonitorUnavailable =>
188+
LastError is int errorCode && DdcErrorClassifier.IsPhysicalMonitorUnavailable(errorCode);
205189

206190
public static VcpProbeObservation Success(
207191
byte code,

0 commit comments

Comments
 (0)