Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Component Project Naming Conventions #64

Closed
shaggygi opened this issue Dec 2, 2018 · 17 comments
Closed

Component Project Naming Conventions #64

shaggygi opened this issue Dec 2, 2018 · 17 comments
Labels
Design Discussion Ongoing discussion about design without consensus

Comments

@shaggygi
Copy link
Contributor

shaggygi commented Dec 2, 2018

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.
  1. 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
Copy link
Member

joshfree commented Dec 4, 2018

@terrajobst @KrzysztofCwalina any thoughts?

@joshfree joshfree added the Design Discussion Ongoing discussion about design without consensus label Dec 4, 2018
@KrzysztofCwalina
Copy link
Member

Tca9548A, Mcp3008 are the right casing.

@shaggygi
Copy link
Contributor Author

shaggygi commented Dec 5, 2018

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
Copy link
Member

joperezr commented Dec 5, 2018

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
Copy link
Member

joshfree commented Dec 5, 2018

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
Copy link
Contributor Author

shaggygi commented Dec 5, 2018

@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
Copy link
Contributor Author

shaggygi commented Dec 6, 2018

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
Copy link
Member

joperezr commented Dec 6, 2018

@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
Copy link
Contributor Author

shaggygi commented Dec 6, 2018

@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
Copy link
Member

joperezr commented Dec 6, 2018

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
Copy link
Member

@joperezr so would it be okay to update those?

You mean the code, not the guidelines, right?

@joperezr
Copy link
Member

joperezr commented Dec 6, 2018

Yup, he means the code :)

@shaggygi
Copy link
Contributor Author

shaggygi commented Dec 7, 2018

@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
Copy link
Contributor Author

shaggygi commented Dec 7, 2018

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 shaggygi changed the title [Question] Component Project Naming Convention Component Project Naming Conventions Dec 7, 2018
@shaggygi
Copy link
Contributor Author

shaggygi commented Jan 1, 2019

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
Copy link
Member

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
Copy link
Contributor Author

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

joperezr pushed a commit to joperezr/iot that referenced this issue Feb 19, 2019
* Add Fedora 29 Dockerfile with procps for pkill/ps

Remove azure-cli and npm installs: this fails, and I can't find them being used.

Remove NSS upgrade: NSS isn't installed/used by default in 29.

* Remove EOL Fedora 24, and near-EOL 27

* Add npm permission and clean fixes to opensuse
@ghost ghost locked as resolved and limited conversation to collaborators Oct 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Design Discussion Ongoing discussion about design without consensus
Projects
None yet
Development

No branches or pull requests

5 participants