dotnet / iot

This repo includes .NET Core implementations for various IoT boards, chips, displays and PCBs.
MIT License
2.18k stars 586 forks source link

Component Project Naming Conventions #64

Closed shaggygi closed 5 years ago

shaggygi commented 5 years ago

This is related to #55, but created separate issue to understand more specifics.

Topic Adding more libraries that control specific sensors or devices

  1. What is the appropriate naming convention for component projects? Camel-casing sometimes messes with me when it comes to hardware and acronyms. 😄

    • e.g. MCP3008 or Mcp3008, etc.
    • e.g. TCA9548A, Tca9548A, Tca9548a, etc.
  2. Should we create projects with top-level namespace specific to component name or should it be under a particular format?

    • e.g. MCP3008. Namespace = Mcp3008

I personally like the component name as top-level as it is simple, but I assume there are other (potential) APIs in the wild that users have named the same. If they did and already have NuGet packages, what impact would that have when we release the repo's related API package? For example, Microsoft.IoT.AdcMcp3008 is not the same name, but had potential to be released as Mcp3008.

joshfree commented 5 years ago

@terrajobst @KrzysztofCwalina any thoughts?

KrzysztofCwalina commented 5 years ago

Tca9548A, Mcp3008 are the right casing.

shaggygi commented 5 years ago

There are many component datasheets that cover a family of devices. Usually there is a consistent set of characters that represent a component name and some characters that change representing different capabilities. Use Microchip 24xx512 EEPROM as example. This datasheet covers 24AA512, 24LC512 and 24FC512 devices. The middle letters represent voltage and frequency ranges for this component.

In addition, many times a vendor will use the letter 'X' to represent the varying characters of component name. The datasheet link above references 24XX512. So would the convention for this binding project name be...

Note: I'm assuming we would have to start the name with a letter since a number is invalid. I used 'M' (for Microchip) in this case.

M24Aa512 (Uses base component name and expects user to know it covers other component flavors) M24512 (This could have potential of having another component name in the wild) M24XX512 M24xx512 M24Xx512 (This seems like the choice based on @KrzysztofCwalina feedback)

Thx

joperezr commented 5 years ago

I don't have a strong opinion with casing. For namespaces though, I believe that @richlander did envision us having all bindings under the same namespace "Iot.Device" like Mcp3008 is today.

joshfree commented 5 years ago

Thanks for starting this discussion @shaggygi. I wonder with this concrete example if instead of having a unique class for each Microchip I2C Serial EEPROM device (M24AA01, M24AA02, M24AA04, M24AA16, M24AA32A, M24AA64, M24AA128, M24AA256, M25AA512, ...) if instead it would make sense to call it something like "MicrochipI2cSerialEeprom" - which encapsulates all M24XX bindings; and then also have "MicrochipSpiSerialEeprom", "MicrochipSingleWireSerialEeprom", ... for the other class of Microchip EEPROM devices.

Just a thought, and maybe it's wrong; and certainly we can also refactor and consolidate device bindings as we get going. After all we only have a single device binding in this repo so far..

shaggygi commented 5 years ago

@joshfree

I don't think it would be a good idea to create a binding class for every EEPROM device either. But instead, possibly a binding for a group/family (based on datasheet/similar). In my example above , there would be a binding API with 1 class M24Xx512 (or similar naming) that included specific functionality to that family. Therefore, you could instantiate an object from M24Xx512 class and still control all the components under that group. The reasoning is that family most likely has certain characteristics like control registers/bits, timing, pins, etc. that is most likely different compared to the next group/family datasheet. In other words, there might be another Microchip I2C EEPROM family that would have its own binding API and 1 class (e.g. M25BLAH89) to support all its characteristics/interactions.

We do this already with the Mcp3008 binding as it can control a 4 channel (MCP3004) or the 8 channel (MCP3008). It is up to the dev to make sure they know the actual device they're working with and what channels are available.

We could prototype your idea and review. I will try to research a few similar I2C/SPI EEPROM to add within one binding. When it comes to including many types of groups within one binding API (e.g. MicrochipSpiSerialEeprom), does that increase memory size and such for projects? I'm assuming RPi would have enough space, but what if you had many bindings (or other processes running), would this be a strain on an IoT device? Not sure.

shaggygi commented 5 years ago

NOTE: This is a side questions, but still related to code conventions.

While working on a binding, I am using the Mcp3008 binding as an example. I noticed a couple of things I wanted to make sure I'm coding correctly going forward.

There are a few fields that are uppercase (e.g. _CLK, _MISO, etc.). Is this the appropriate style or should it be lowercase (e.g. _clk, _miso, etc.).

I also noticed the underscore being used in parameters and variables throughout code. Use "adc_channel" and "trim_pot" as example. Is it better to separate the words with underscore or should it be the usual casing like "adcChannel" and "trimPot"?

Sorry to be too picky and asking too many questions. Just want to make sure before proceeding. Thx

@KrzysztofCwalina Thoughts?

joperezr commented 5 years ago

@shaggygi we use the same coding style as the one enforced in corefx: https://github.com/dotnet/corefx/blob/master/Documentation/coding-guidelines/coding-style.md

That means that you are right, those fields should be _clk, _miso etc. I believe the only reason why they were added with that casing is that they were a port from an Adafruit project but we didn't make sure that the variables were renamed to reflect our coding style. Please do follow the corefx guidelines when writing your own binding.

shaggygi commented 5 years ago

@joperezr so would it be okay to update those? I plan to add a little more cleanup on that project. I can do at the same time.

joperezr commented 5 years ago

That would be awesome, thank you very much. If for whatever reason you don't get to it don't worry, I'll go back and fix those.

terrajobst commented 5 years ago

@joperezr so would it be okay to update those?

You mean the code, not the guidelines, right?

joperezr commented 5 years ago

Yup, he means the code :)

shaggygi commented 5 years ago

@joperezr PR just created. Please be gentle, but honest (and patient) with me. 😄 I just want to make this a good story. Also, I did basic testing with SPI/GPIO and single/diff channel types, but nothing extensive. I need to get my workstation setup better with test equipment... maybe then I can double-check all channels work as expected. Thx

shaggygi commented 5 years ago

NOTE: This is a yet another side questions, but still related to code conventions.

When cleaning up the Mcp3008 binding, I wanted to create more files. For example, I wanted to separate out CommunicationProtocol and new InputConfiguration enums to their own file. Understanding Mcp3008 binding is simple, I can see where it would be a good practice. But where I'm more concerned is when there are complex bindings that will need to have more folders/files in addition to the [binding].cs file.

When I tried to separate and add the new InputConfiguration.cs file, I wasn't sure what the namespace be for the file. It shouldn't be Iot.Device like Mcp3008 class as there could be similar separate files/classes created in future bindings.

I tried using the namespace Iot.Device.Mcp3008 for InputConfiguration.cs file, but got errors stating Mcp3008 was already used... which is understandable.

So the question is what is the best practice on namespace (or alias) when create more files (class, enums, etc.) in addition to the main binding class file?

Thx

shaggygi commented 5 years ago

NOTE: This is a yet another side questions, but still related to code conventions.

Bindings are usually going to have groups of things like registers or commands. What is the guidance for such things related to namespaces? Singular or plural?

MyBinding
  Command
    DoSomething.cs
    ...
  Register
    RegA.cs
    ...

// Or...

MyBinding
  Commands
    DoSomething.cs
    ...
  Registers
    RegA.cs
    ...
joperezr commented 5 years ago

For that question I believe we have been using plural. I was looking at this thread and it seems like all the concerns/questions around naming have been answered, so I'll close this issue for now. @shaggygi if you still see any issues or have any questions around naming feel free to reopen this.

shaggygi commented 5 years ago

@joperezr Yes, I think I'm good on this for now. Thx