This proposal is a collection of different modifications and grouped here as there are many breaking changes being proposed. The intent is to review, decide and update (if needed) namespaces, class names and file locations in a consistent structure between the different types of interfaces within System.Device.Gpio API.
This seems to fit with @terrajobst comments during this design review, as well.
Since System.Device.Gpio is in early previews, it seems like the opportune time to review and decide how to structure the core components. This will reduce breaking changes in the future.
The proposed changes are to rename and move files as shown in the folder structure below. To simplify, this attempts to point out only the changes for the files being modified.
Gpio
Drivers
GpioDriver.cs
HummingBoardGpioDriver.cs
HummingBoardGpioDriver.Linux.cs
HummingBoardGpioDriver.Windows.cs
RaspberryPi3GpioDriver.cs
RaspberryPi3GpioDriver.Linux.cs
RaspberryPi3GpioDriver.Windows.cs
UnixGpioDriver.Linux.cs
UnixGpioDriver.Windows.cs
UnixGpioDriverPin.Linux.cs // Also simplified by removing Device
Windows10GpioDriver.Linux.cs
Windows10GpioDriver.Windows.cs
Windows10GpioDriverPin.Windows.cs
GpioPinChangeEventHandler.cs
GpioPinEventTypes.cs
GpioPinMode.cs
GpioPinNumberingScheme.cs
GpioPinValue.cs
GpioPinValueChangedEventArgs.cs
See #118 and #534 for related details
I2c
Devices
I2cDevice.cs
I2cDevice.Linux.cs
I2cDevice.Windows.cs
[other devices here that were not moved or renamed]
I2cConnectionSettings.cs (didn't change; showing for location purposes)
See #204 for related details
Pwm
Drivers
PwmDriver.cs
See #119 and #534 for related details
Spi
Devices
SpiDevice.cs
SpiDevice.Linux.cs
SpiDevice.Windows.cs
[other devices here that were not moved or renamed]
SpiConnectionSettings.cs (didn't change; showing for location purposes)
@joshfree @joperezr Just wanted to ping this one again now that Preview 2 is out.
Hey @shaggygi thanks for bringing up all of these good questions. I do agree that there are some inconsistencies across our library today, which should get fixed before we ship a stable package. For example, I don't like the fact that Gpio and PWM have a controller which is what you interact with and don't have a specific device object (which in this case would be a GpioPin or PwmPin) but in I2c and Spi we do have the individual devices and don't have a controller for the whole bus. I also think we are lacking some behavior interfaces which should be added which those are tracked by other issues.
In general for your other questions, I don't think that adding Gpio into all of the classnames under Gpio makes sense as since you point out, it does add redundancy when you see the fully qualified name. That said, we do have some counter-examples for that. Let's keep this issue around and use it to discuss concrete examples that should change for the next previews so that we can make things more consistent.
Sounds good. It might be worth to keep some action items (checkboxes) on this as the main place to confirm the overall API changes that need to happen to call it complete (even if there are other issues created for specific action items).
I agree the namespaces help separate out (although preferred to include). I just want to make sure (if possible) to make different sections close that make sense. As noted... I'm okay with System.Device.Gpio.UnixDriver, but we should also have System.Device.Pwm.UnixDriver.
And while using the namespace to separate will help, I'm more worried when we begin working on future interfaces and implement bindings that have multiple functions. For example, now that the IGpioController is pushed, I plan to begin working on a new binding that provides GPIO and PWM. The plan is to later add what is pushed for IPwmController (or related solution). By not having the function in name, it might get a little confusing like the UnixDriver example as we could be using both.
Thanks again for managing this... it's a booger :smile:
@joperezr we might need to get focused on this one soon. Thoughts?
Agreed, all of the issues that are flagged today as 3.0 are things we have to finish in the next few weeks, so we will definitely get to this one in the next couple of weeks.
@joperezr I updated the I2C and SPI parts in initial proposal above. Hope to have more details on #118 and #119 soon. And even if those proposals don't get completed by v1 (which I hope), the related changes in this proposal could be updated with or without and won't affect them.
I saw the comment on the I2c proposal about this. I think having that change is fine.
@joperezr @krwq Now that I2C and SPI is winding down, would it be possible to go ahead and put in a PR for updating the related Drivers > Devices folders and namespaces? I could probably complete those 2 parts of this issue over the weekend.
Related to those, the only thing I had a question on was moving the xxxDevice.cs to the folder. I like those being grouped with the other ...Device.cs files, but on the fence keeping as is. And if they are moved in Devices folder, should the namespace be updated for xxxDevice.cs, as well?
@shaggygi do you mean specifically those 3:
@joperezr are you ok with that? It looks fine with me
regardless - could you make sure to do any file renames (+ csproj adjustments) in a separate commit to make it easier to review?
@krwq yup, those are the 3 things to focus on for both. I can do separate PRs for each type if that helps break down since this is breaking changes.
@shaggygi assuming @joperezr agrees with this work I think ideally I'd at minimum do 2 PRs or more depending on the diff size:
cc: @richlander - TL;DR; we're thinking of doing some renaming for the sake of consistency - this matches with previous API review feedback
@shaggygi those three changes that @krwq mentioned above are approved.
I suppose that the PWM PR that we have open is the last thing about this issue that we want to fix? @shaggygi @krwq
@joperezr Still need to move the I2c/Spi Device classes back to their root (based on PWM discussion), but need to finalize #569 before I can update.
I think GPIO still needs a quick review. For example...
So for the ones you mentioned, I2c and Spi classes moving won't need to happen before 3.0, because this only involves the specific platform devices, which are internal, so no problem if we don't do that for now.
Same goes for the other cases, UnixDriverDevicePin and LibgpiodDriverEventHandler are internal, so no problem if we want to rename/move them later.
Because of that, I will move this issue to Future for our internal tracking, but feel free to push back if you think otherwise.
Most helpful comment
@shaggygi those three changes that @krwq mentioned above are approved.