Update samples using I2c to work on Windows

[krwq]

Fixes: #498

This won't build until we get new package version

This incorporates changes done here: #505

[shaggygi]

LGTM. You beat me to it 😄

[krwq]

@shaggygi I'll leave you SPI 😄

[joperezr]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[joperezr]

Just as a comment, now that we have I2cDevice.Create method, we might want to think again how most bindings take a I2cDevice as part of their constructor, such that insted they would just create a new one inside their class. There were only two reasons to have it passed in: support for windows/unix (which we no longer need) and in order to allow passing in a device that is not a Unix or Windows I2c device, but instead something that implementes the II2cDevice interface, like a I2C extender.

[shaggygi]

Could the guidance be to pass in I2cDevice? or SpiDevice? similar to IGpioController. As you mentioned, I could see other flavors (i. e. GpioI2cDevice, GpioSpiDevice) in the future.

Inside the binding constructor, have something like...

_i2cDevice = I2cDevice ?? I2cDevice.Create(settings);
_spiDevice = SpiDevice ?? SpiDevice.Create(settings);

Also, with guidance showing the nullable types to pass... could hopefully trigger binding devs to code the binding API with the new C# 8 feature.

[krwq]

I'd refrain from that for now unless it covers 95% of the cases. Example thing what I did for SenseHat:

• Lsm9Ds1AccelerometerAndGyroscope doesn't know anything about I2C addresses
• SenseHatAccelerometerAndGyroscope thin wrapper on top of Lsm9Ds1AccelerometerAndGyroscope which only defaults I2C address since it's constant in context of SenseHat

Ideally I think we should create an issue and set up guidelines for this but I think this is non-issue at this point and regardless of this issue we need at least one constructor which takes already existing I2cDevice object.

I don't personally like default value as the ownership is not immediately obvious. Having two classes one which always default and one which never does makes stuff much clearer

[krwq]

I'd prefer if we made this non-nullable and create second implementation like DefaultXyzDevice which just inherits, creates I2cDevice and disposes of it. This way you got covered 90% of the cases and advanced usage

[joperezr]

How did you find all these places? Did you just do a search on all places where we were doing a new UnixI2cDevice( or new WindowsI2cDevice ?

[krwq]

@joperezr almost. I searched for UnixI2cDevice and WindowsI2cDevice (no new) in src/devices and samples and manually reviewed the code if it needs some more adjustments. Find & replace covers 90% though

[joperezr]

Why only src/devices? should we do it on the root as well in case we were doing this in the tools or root samples?

[shaggygi]

Yes, I was just looking at DeviceApiTester and will update those, as well with var or I2cDevice/SpiDevice

[krwq]

@joperezr I meant root samples when I said samples. Reason was to avoid the hits from the implementation of I2cDevice itself which I didn't expect to have anything useful anyway

[shaggygi]

@krwq @joperezr I get local build errors now with the following package added in System.Device.Gpio.csproj. All projects build in iot directory after I remove it. Thoughts?

<PackageReference Include="Microsoft.DotNet.GenAPI" Version="$(MicrosoftDotNetGenApiPackageVersion)">
      <PrivateAssets>all</PrivateAssets>
    </PackageReference>

[joperezr]

@shaggygi do you mind sharing a binlog of the error you are seeing so that I can take a better look? In order to produce one, just run msbuild System.Device.Gpio.csproj /t:rebuild /bl:msbuild.binlog on a developer command prompt which will generate a file called msbuild.binlog.

[shaggygi]

@joperezr Does this work? Just remove the .zip.
msbuild.binlog.zip

[joperezr]

Does this work? Just remove the .zip.

mmm that is very weird, does this repro if you are on a clean master? try doing:

git pull
git reset --hard
git clean -fdx
build.cmd

This will get latest changes, remove all pending changes, clean up your enlistment and run build again.

[krwq]

also if you cancelled build during restore it might be worth to clean nuget cache ($HOME/.nuget or %userprofile%\.nuget)

[shaggygi]

git pull
git reset --hard
git clean -fdx
build.cmd

This appears to have fix the error. One thing I noticed was a version of preview6 of something (in the output text) where this has preview5. Not sure if was SDK, runtime, etc. 😕

Thx