Skip to content

Commit 0b7733e

Browse files
Modern code review (#1263)
* Code cleanup * Update projects * Project updates * Fix breaks * Update ExplorerHat project * Update Ft4222 project * Update GoPiGo3 * Update GrovePi project * Update a few more projects * More updates * Update null checks * Fix ArgumentException usage * Apply suggestions from code review Co-authored-by: Marc Brooks <IDisposable@gmail.com> * Update src/devices/GoPiGo3/GoPiGo3.cs * Update src/devices/Ina219/Ina219.cs Co-authored-by: Marc Brooks <IDisposable@gmail.com> * Switch exception type * Switch exception type * Update src/devices/Mcp3428/Mcp342x.cs Co-authored-by: Marc Brooks <IDisposable@gmail.com> * Update src/devices/Mcp3xxx/Mcp3xxx.cs Co-authored-by: Marc Brooks <IDisposable@gmail.com> * Update src/devices/Mcp9808/Mcp9808.cs Co-authored-by: Marc Brooks <IDisposable@gmail.com> * Update src/devices/Mlx90614/Mlx90614.cs Co-authored-by: Marc Brooks <IDisposable@gmail.com> * Update src/devices/Ssd13xx/Ssd13xx.cs Co-authored-by: Marc Brooks <IDisposable@gmail.com> * Update src/devices/Tcs3472x/Tcs3472x.cs Co-authored-by: Marc Brooks <IDisposable@gmail.com> * Update src/devices/Vl53L0X/Vl53L0X.cs Co-authored-by: Marc Brooks <IDisposable@gmail.com> * Update src/devices/Ws28xx/Ws28xx.cs Co-authored-by: Marc Brooks <IDisposable@gmail.com> * Update src/devices/Hmc5883l/Hmc5883l.cs Co-authored-by: Marc Brooks <IDisposable@gmail.com> * Update src/devices/Mpr121/Mpr121.cs Co-authored-by: Marc Brooks <IDisposable@gmail.com> * Update src/devices/Mpu9250/Mpu6500.cs Co-authored-by: Marc Brooks <IDisposable@gmail.com> * Update src/devices/Nrf24l01/Nrf24l01.cs Co-authored-by: Marc Brooks <IDisposable@gmail.com> * Update src/devices/Pn5180/Pn5180.cs Co-authored-by: Marc Brooks <IDisposable@gmail.com> * Update src/devices/Pn532/Pn532.cs Co-authored-by: Marc Brooks <IDisposable@gmail.com> * Update src/devices/Pn532/Pn532.cs Co-authored-by: Marc Brooks <IDisposable@gmail.com> * Update src/devices/RadioTransmitter/Devices/Kt0803/Kt0803.cs Co-authored-by: Marc Brooks <IDisposable@gmail.com> * Update src/devices/RadioReceiver/Devices/Tea5767/Tea5767.cs Co-authored-by: Marc Brooks <IDisposable@gmail.com> * Update src/devices/Rtc/Devices/Ds1307/Ds1307.cs Co-authored-by: Marc Brooks <IDisposable@gmail.com> * Update src/devices/Rtc/Devices/Ds3231/Ds3231.cs Co-authored-by: Marc Brooks <IDisposable@gmail.com> * Update src/devices/Rtc/Devices/Pcf8563/Pcf8563.cs Co-authored-by: Marc Brooks <IDisposable@gmail.com> * Update src/devices/Seesaw/SeesawBase.cs Co-authored-by: Marc Brooks <IDisposable@gmail.com> * Update src/devices/ShiftRegister/ShiftRegister.cs Co-authored-by: Marc Brooks <IDisposable@gmail.com> * Update src/devices/Shtc3/Shtc3.cs Co-authored-by: Marc Brooks <IDisposable@gmail.com> * Update src/devices/Si7021/Si7021.cs Co-authored-by: Marc Brooks <IDisposable@gmail.com> * Update GoPiGo3.cs * Update per feedback * Add type in place of var * Revert one switch expression to switch statement * Add pragma for []? Co-authored-by: Marc Brooks <IDisposable@gmail.com>
1 parent eb7163d commit 0b7733e

File tree

267 files changed

+2211
-3648
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

267 files changed

+2211
-3648
lines changed

Diff for: src/devices/Ads1115/Ads1115.cs

+42-86
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@ public class Ads1115 : IDisposable
4747
/// <param name="measuringRange">Programmable Gain Amplifier</param>
4848
/// <param name="dataRate">Data Rate</param>
4949
/// <param name="deviceMode">Initial device mode</param>
50-
public Ads1115(I2cDevice i2cDevice, InputMultiplexer inputMultiplexer = InputMultiplexer.AIN0, MeasuringRange measuringRange = MeasuringRange.FS4096, DataRate dataRate = DataRate.SPS128,
51-
DeviceMode deviceMode = DeviceMode.Continuous)
50+
public Ads1115(I2cDevice i2cDevice, InputMultiplexer inputMultiplexer = InputMultiplexer.AIN0, MeasuringRange measuringRange = MeasuringRange.FS4096,
51+
DataRate dataRate = DataRate.SPS128, DeviceMode deviceMode = DeviceMode.Continuous)
5252
{
5353
_i2cDevice = i2cDevice ?? throw new ArgumentNullException(nameof(i2cDevice));
5454
_inputMultiplexer = inputMultiplexer;
@@ -61,7 +61,6 @@ public Ads1115(I2cDevice i2cDevice, InputMultiplexer inputMultiplexer = InputMul
6161
_comparatorPolarity = ComparatorPolarity.Low;
6262
_comparatorLatching = ComparatorLatching.NonLatching;
6363
_comparatorQueue = ComparatorQueue.Disable;
64-
_shouldDispose = false;
6564

6665
SetConfig();
6766
DisableAlertReadyPin();
@@ -79,18 +78,18 @@ public Ads1115(I2cDevice i2cDevice, InputMultiplexer inputMultiplexer = InputMul
7978
/// <param name="dataRate">Data Rate</param>
8079
/// <param name="deviceMode">Initial device mode</param>
8180
public Ads1115(I2cDevice i2cDevice,
82-
GpioController gpioController, int gpioInterruptPin, bool shouldDispose = true, InputMultiplexer inputMultiplexer = InputMultiplexer.AIN0, MeasuringRange measuringRange = MeasuringRange.FS4096, DataRate dataRate = DataRate.SPS128,
83-
DeviceMode deviceMode = DeviceMode.Continuous)
81+
GpioController? gpioController, int gpioInterruptPin, bool shouldDispose = true, InputMultiplexer inputMultiplexer = InputMultiplexer.AIN0, MeasuringRange measuringRange = MeasuringRange.FS4096,
82+
DataRate dataRate = DataRate.SPS128, DeviceMode deviceMode = DeviceMode.Continuous)
8483
: this(i2cDevice, inputMultiplexer, measuringRange, dataRate, deviceMode)
8584
{
86-
_gpioController = gpioController ?? throw new ArgumentNullException(nameof(gpioController));
87-
if (gpioInterruptPin < 0 || gpioInterruptPin >= gpioController.PinCount)
85+
_gpioController = gpioController ?? new GpioController();
86+
if (gpioInterruptPin < 0 || gpioInterruptPin >= _gpioController.PinCount)
8887
{
8988
throw new ArgumentOutOfRangeException(nameof(gpioInterruptPin), $"The given GPIO Controller has no pin number {gpioInterruptPin}");
9089
}
9190

9291
_gpioInterruptPin = gpioInterruptPin;
93-
_shouldDispose = shouldDispose;
92+
_shouldDispose = shouldDispose || gpioController is null;
9493
}
9594

9695
/// <summary>
@@ -164,11 +163,7 @@ public DeviceMode DeviceMode
164163
/// Comparator mode.
165164
/// Only relevant if the comparator trigger event is set up and is changed by <see cref="EnableComparator(short, short, ComparatorMode, ComparatorQueue)"/>.
166165
/// </summary>
167-
public ComparatorMode ComparatorMode
168-
{
169-
get;
170-
private set;
171-
}
166+
public ComparatorMode ComparatorMode { get; private set; }
172167

173168
/// <summary>
174169
/// Comparator polarity. Indicates whether the rising or the falling edge of the ALRT/RDY Pin is relevant.
@@ -208,13 +203,7 @@ public ComparatorLatching ComparatorLatching
208203
/// Minimum number of samples exceeding the lower/upper threshold before the ALRT pin is asserted.
209204
/// This can only be set with <see cref="EnableComparator(short, short, ComparatorMode, ComparatorQueue)"/>.
210205
/// </summary>
211-
public ComparatorQueue ComparatorQueue
212-
{
213-
get
214-
{
215-
return _comparatorQueue;
216-
}
217-
}
206+
public ComparatorQueue ComparatorQueue => _comparatorQueue;
218207

219208
/// <summary>
220209
/// This event fires when a new value is available (in conversion ready mode) or the comparator threshold is exceeded.
@@ -285,7 +274,7 @@ private void DisableAlertReadyPin()
285274
(byte)Register.ADC_CONFIG_REG_HI_THRESH, 0x7F, 0xFF
286275
};
287276
_i2cDevice.Write(writeBuff);
288-
if (_gpioController != null)
277+
if (_gpioController is object)
289278
{
290279
_gpioController.UnregisterCallbackForPinValueChangedEvent(_gpioInterruptPin, ConversionReadyCallback);
291280
_gpioController.ClosePin(_gpioInterruptPin);
@@ -319,7 +308,7 @@ private void WriteComparatorRegisters(short loThreshold, short hiThreshold)
319308
/// for interrupt handling.</exception>
320309
public void EnableConversionReady()
321310
{
322-
if (_gpioController == null)
311+
if (_gpioController is null)
323312
{
324313
throw new InvalidOperationException("Must have provided a GPIO Controller for interrupt handling.");
325314
}
@@ -349,7 +338,7 @@ public void EnableConversionReady()
349338

350339
private void ConversionReadyCallback(object sender, PinValueChangedEventArgs pinValueChangedEventArgs)
351340
{
352-
if (AlertReadyAsserted != null)
341+
if (AlertReadyAsserted is object)
353342
{
354343
AlertReadyAsserted();
355344
}
@@ -368,10 +357,7 @@ private void ConversionReadyCallback(object sender, PinValueChangedEventArgs pin
368357
/// <param name="queueLength">Minimum number of samples that must exceed the threshold to trigger the event</param>
369358
/// <exception cref="InvalidOperationException">The GPIO Controller for the interrupt handler has not been set up</exception>
370359
public void EnableComparator(ElectricPotential lowerValue, ElectricPotential upperValue, ComparatorMode mode,
371-
ComparatorQueue queueLength)
372-
{
373-
EnableComparator(VoltageToRaw(lowerValue), VoltageToRaw(upperValue), mode, queueLength);
374-
}
360+
ComparatorQueue queueLength) => EnableComparator(VoltageToRaw(lowerValue), VoltageToRaw(upperValue), mode, queueLength);
375361

376362
/// <summary>
377363
/// Enable comparator callback mode.
@@ -388,7 +374,7 @@ public void EnableComparator(ElectricPotential lowerValue, ElectricPotential upp
388374
public void EnableComparator(short lowerValue, short upperValue, ComparatorMode mode,
389375
ComparatorQueue queueLength)
390376
{
391-
if (_gpioController == null)
377+
if (_gpioController is null)
392378
{
393379
throw new InvalidOperationException("GPIO Controller must have been provided in constructor for this operation to work");
394380
}
@@ -444,7 +430,7 @@ private ushort ReadConfigRegister()
444430
/// This method must only be called in powerdown mode, otherwise it would timeout, since the busy bit never changes.
445431
/// Due to that, we always write the configuration first in power down mode and then enable the continuous bit.
446432
/// </summary>
447-
/// <exception cref="TimeoutException">A timeout occured waiting for the ADC to finish the conversion</exception>
433+
/// <exception cref="TimeoutException">A timeout occurred waiting for the ADC to finish the conversion</exception>
448434
private void WaitWhileBusy()
449435
{
450436
// In powerdown-mode, wait until the busy bit goes high
@@ -502,10 +488,7 @@ private short ReadRawInternal()
502488
/// For performance reasons, it is advised to use this method if quick readings with different input channels are required,
503489
/// instead of setting all the properties first and then calling <see cref="ReadRaw()"/>.
504490
/// </remarks>
505-
public short ReadRaw(InputMultiplexer inputMultiplexer)
506-
{
507-
return ReadRaw(inputMultiplexer, MeasuringRange, DataRate);
508-
}
491+
public short ReadRaw(InputMultiplexer inputMultiplexer) => ReadRaw(inputMultiplexer, MeasuringRange, DataRate);
509492

510493
/// <summary>
511494
/// Reads the next raw value, first switching to the given input and ranges.
@@ -590,30 +573,16 @@ public short VoltageToRaw(ElectricPotential voltage)
590573
/// <returns>An electric potential (voltage).</returns>
591574
public ElectricPotential MaxVoltageFromMeasuringRange(MeasuringRange measuringRange)
592575
{
593-
double voltage;
594-
switch (measuringRange)
595-
{
596-
case MeasuringRange.FS6144:
597-
voltage = 6.144;
598-
break;
599-
case MeasuringRange.FS4096:
600-
voltage = 4.096;
601-
break;
602-
case MeasuringRange.FS2048:
603-
voltage = 2.048;
604-
break;
605-
case MeasuringRange.FS1024:
606-
voltage = 1.024;
607-
break;
608-
case MeasuringRange.FS0512:
609-
voltage = 0.512;
610-
break;
611-
case MeasuringRange.FS0256:
612-
voltage = 0.256;
613-
break;
614-
default:
615-
throw new ArgumentOutOfRangeException(nameof(measuringRange), "Unknown measuring range used");
616-
}
576+
double voltage = measuringRange switch
577+
{
578+
MeasuringRange.FS6144 => 6.144,
579+
MeasuringRange.FS4096 => 4.096,
580+
MeasuringRange.FS2048 => 2.048,
581+
MeasuringRange.FS1024 => 1.024,
582+
MeasuringRange.FS0512 => 0.512,
583+
MeasuringRange.FS0256 => 0.256,
584+
_ => throw new ArgumentOutOfRangeException(nameof(measuringRange), "Unknown measuring range used")
585+
};
617586

618587
return ElectricPotential.FromVolts(voltage);
619588
}
@@ -623,50 +592,37 @@ public ElectricPotential MaxVoltageFromMeasuringRange(MeasuringRange measuringRa
623592
/// </summary>
624593
/// <param name="dataRate">One of the <see cref="DataRate"/> enumeration members.</param>
625594
/// <returns>A frequency, in Hertz</returns>
626-
public double FrequencyFromDataRate(DataRate dataRate)
595+
public double FrequencyFromDataRate(DataRate dataRate) => dataRate switch
627596
{
628-
switch (dataRate)
629-
{
630-
case DataRate.SPS008:
631-
return 8.0;
632-
case DataRate.SPS016:
633-
return 16.0;
634-
case DataRate.SPS032:
635-
return 32.0;
636-
case DataRate.SPS064:
637-
return 64.0;
638-
case DataRate.SPS128:
639-
return 128.0;
640-
case DataRate.SPS250:
641-
return 250.0;
642-
case DataRate.SPS475:
643-
return 475.0;
644-
case DataRate.SPS860:
645-
return 860.0;
646-
default:
647-
throw new ArgumentOutOfRangeException(nameof(dataRate), "Unknown data rate used");
648-
}
649-
}
597+
DataRate.SPS008 => 8.0,
598+
DataRate.SPS016 => 16.0,
599+
DataRate.SPS032 => 32.0,
600+
DataRate.SPS064 => 64.0,
601+
DataRate.SPS128 => 128.0,
602+
DataRate.SPS250 => 250.0,
603+
DataRate.SPS475 => 475.0,
604+
DataRate.SPS860 => 860.0,
605+
_ => throw new ArgumentOutOfRangeException(nameof(dataRate), "Unknown data rate used")
606+
};
650607

651608
/// <summary>
652609
/// Cleanup.
653610
/// Failing to dispose this class, especially when callbacks are active, may lead to undefined behavior.
654611
/// </summary>
655612
public void Dispose()
656613
{
657-
if (_i2cDevice != null)
614+
if (_i2cDevice is object)
658615
{
659616
DisableAlertReadyPin();
660-
_i2cDevice?.Dispose();
617+
_i2cDevice.Dispose();
661618
_i2cDevice = null!;
662619
}
663620

664-
if (_shouldDispose && _gpioController != null)
621+
if (_shouldDispose)
665622
{
666-
_gpioController.Dispose();
623+
_gpioController?.Dispose();
624+
_gpioController = null;
667625
}
668-
669-
_gpioController = null;
670626
}
671627
}
672628
}

Diff for: src/devices/Adxl345/Adxl345.cs

+9-20
Original file line numberDiff line numberDiff line change
@@ -45,27 +45,16 @@ public class Adxl345 : IDisposable
4545
/// <param name="gravityRange">Gravity Measurement Range</param>
4646
public Adxl345(SpiDevice sensor, GravityRange gravityRange)
4747
{
48-
if (gravityRange == GravityRange.Range02)
48+
_sensor = sensor ?? throw new ArgumentNullException(nameof(sensor));
49+
_range = gravityRange switch
4950
{
50-
_range = 4;
51-
}
52-
else if (gravityRange == GravityRange.Range04)
53-
{
54-
_range = 8;
55-
}
56-
else if (gravityRange == GravityRange.Range08)
57-
{
58-
_range = 16;
59-
}
60-
else if (gravityRange == GravityRange.Range16)
61-
{
62-
_range = 32;
63-
}
64-
51+
GravityRange.Range02 => 4,
52+
GravityRange.Range04 => 8,
53+
GravityRange.Range08 => 16,
54+
GravityRange.Range16 => 32,
55+
_ => 0
56+
};
6557
_gravityRangeByte = (byte)gravityRange;
66-
67-
_sensor = sensor;
68-
6958
Initialize();
7059
}
7160

@@ -119,7 +108,7 @@ private Vector3 ReadAcceleration()
119108
/// </summary>
120109
public void Dispose()
121110
{
122-
if (_sensor != null)
111+
if (_sensor is object)
123112
{
124113
_sensor.Dispose();
125114
_sensor = null!;

Diff for: src/devices/Adxl345/samples/Program.cs

+3-4
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,13 @@
99

1010
SpiConnectionSettings settings = new (0, 0)
1111
{
12-
ClockFrequency = Iot.Device.Adxl345.Adxl345.SpiClockFrequency,
13-
Mode = Iot.Device.Adxl345.Adxl345.SpiMode
12+
ClockFrequency = Adxl345.SpiClockFrequency,
13+
Mode = Adxl345.SpiMode
1414
};
1515

1616
using SpiDevice device = SpiDevice.Create(settings);
17-
1817
// set gravity measurement range ±4G
19-
using Iot.Device.Adxl345.Adxl345 sensor = new Iot.Device.Adxl345.Adxl345(device, GravityRange.Range04);
18+
using Adxl345 sensor = new Adxl345(device, GravityRange.Range04);
2019
while (true)
2120
{
2221
// read data

Diff for: src/devices/Ags01db/Ags01db.cs

+16-16
Original file line numberDiff line numberDiff line change
@@ -18,17 +18,17 @@ public class Ags01db : IDisposable
1818
private const byte CRC_INIT = 0xFF;
1919
private I2cDevice _i2cDevice;
2020

21-
private int _lastMeasurment = 0;
21+
private int _lastMeasurement = 0;
2222

2323
/// <summary>
2424
/// ASG01DB VOC (Volatile Organic Compounds) Gas Concentration (ppm)
2525
/// </summary>
26-
public double Concentration { get => GetConcentration(); }
26+
public double Concentration => GetConcentration();
2727

2828
/// <summary>
2929
/// ASG01DB Version
3030
/// </summary>
31-
public byte Version { get => GetVersion(); }
31+
public byte Version => GetVersion();
3232

3333
/// <summary>
3434
/// ASG01DB Default I2C Address
@@ -41,16 +41,7 @@ public class Ags01db : IDisposable
4141
/// <param name="i2cDevice">The I2C device used for communication.</param>
4242
public Ags01db(I2cDevice i2cDevice)
4343
{
44-
_i2cDevice = i2cDevice;
45-
}
46-
47-
/// <summary>
48-
/// Cleanup
49-
/// </summary>
50-
public void Dispose()
51-
{
52-
_i2cDevice?.Dispose();
53-
_i2cDevice = null!;
44+
_i2cDevice = i2cDevice ?? throw new ArgumentNullException(nameof(i2cDevice));
5445
}
5546

5647
/// <summary>
@@ -60,9 +51,9 @@ public void Dispose()
6051
private double GetConcentration()
6152
{
6253
// The time of two measurements should be more than 2s.
63-
while (Environment.TickCount - _lastMeasurment < 2000)
54+
while (Environment.TickCount - _lastMeasurement < 2000)
6455
{
65-
Thread.Sleep(TimeSpan.FromMilliseconds(Environment.TickCount - _lastMeasurment));
56+
Thread.Sleep(Environment.TickCount - _lastMeasurement);
6657
}
6758

6859
// Details in the Datasheet P5
@@ -77,7 +68,7 @@ private double GetConcentration()
7768
_i2cDevice.Write(writeBuff);
7869
_i2cDevice.Read(readBuff);
7970

80-
_lastMeasurment = Environment.TickCount;
71+
_lastMeasurement = Environment.TickCount;
8172

8273
// CRC check error
8374
if (!CheckCrc8(readBuff.Slice(0, 2), 2, readBuff[2]))
@@ -154,5 +145,14 @@ private bool CheckCrc8(ReadOnlySpan<byte> data, int length, byte crc8)
154145
return false;
155146
}
156147
}
148+
149+
/// <summary>
150+
/// Cleanup
151+
/// </summary>
152+
public void Dispose()
153+
{
154+
_i2cDevice?.Dispose();
155+
_i2cDevice = null!;
156+
}
157157
}
158158
}

0 commit comments

Comments
 (0)