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

Expose serial number #558

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Conversation

kurisuD
Copy link

@kurisuD kurisuD commented Jan 23, 2023

Hello Guy,
As discussed in Issue #555 , this pull request adds the ability to obtain the serial number of the board.

The change is implemented in a similar fashion as the code extracting the hardware version. A new function gpioHardwareSerialNumber() is added for this purpose in pigpio.c (kept separated from gpioHardwareRevision() intentionally).

The pull request also includes:

  • update to the tests (which are passing, at least with the hardware I have around to test with).
  • update to the python module
  • update to the documentation

Regarding the documentation update, I must admit not being very familiar with the various formats.
They do compile without warning, but if the update is not appropriate, thanks to let me know.
Also, I have spotted that WVCAP wasn't documented properly in sif.html - I have added the tr corresponding, but left it commented, as I wasn't knowing enough about this specific function to document it properly.

Thanks

@guymcswain
Copy link
Collaborator

Thank you for pulling this against the correct branch!

Since the command you propose simply extracts the serial number from /proc/cpuinfo, I want to make sure you are aware of all sources for this information. There seems to be no consistency across all models, architectures, and possible other stuff. To wit:
$ cat /proc/cpuinfo where normally one expects to find it.
$ cat /sys/firmware/devicetree/base/serial-number for 64-bit architectures.
$ cat /proc/device-tree/serial-number and sometimes it can be found here.

It may be a good idea to research if RPi OS has now landed on a consistent place for this information.

Another question is does this really solve the problem wrt Home Assistant. If the client connects to another pigpiod host then a different serial number is returned. Maybe host address and port number are just as important for the integration problem. I'm sorry I didn't try to understand this earlier.

@kurisuD
Copy link
Author

kurisuD commented Jan 24, 2023

Hello Guy

Thanks for your comments.

I wondered the same about the location where the Serial is supposed to be found from. While there seems to be contradictory information on this topic in various discussions on the web, the official doc mentions /proc/cpuinfo here

That being said, I agree the documentation alone doesn't mean it's always the case, so I had a look at the boards I have online around, all of them have Serial available via /proc/cpuinfo, but... all of them are Raspberry Pi Zero W, and with a mix of kernel 4.19.66 and 5.10.103, somehow (I'll need to look into that...).

I think it's purely a kernel thing, /proc/cpuinfo being populated there; also the output of cpuinfo on the board running 5.10.103 has the following extra line:
Model : Raspberry Pi Zero W Rev 1.1

So, while new models may indeed lack the immediate support of the serial number via /proc/cpuinfo (assuming it must have been the case in the past), I tend to think that the functionality should be supported based on the target of the Raspberry Pi OS project (per the doc, /proc/cpuinfo), rather than on being supported based on how the Serial number information has been alternatively provided in the past. The information available on the web regarding the multiple paths is also possibly remnants of the raspbian times ??

But that's probably a personal and anecdotal opinion, so I let you be the judge of it. Also, maybe you do have a variety of boards / SD cards to test with? Unfortunately, with the current supply crunch, I can't get my hands on any.

As for the question regarding home-assistant, clearly in the optics of connecting to the pigpiod daemon remotely, the hostname and port are required as a first step.

The point I think they are making regarding the unique IDs is for them to not rely solely on the hostname of an network device (in this specific case) for their device and entities registries, allowing them to maintain consistency of settings and statistics within a number of potential environmental changes surrounding the home-assistant instance (think dhcp and/or discovery protocols).

For the sake of a full disclosure, my pull request is initiated in the context of the development of an integration for home-assistant, and I'm fairly convinced that their objectives clearly has some solid ground.

Regarding the client connecting to a different host and getting a different serial number, the way I understand how it works is that each board you configure gets its own instance of the integration in a different async loop, i.e. you don't have a single class polling each board one after the other. In other words, it shouldn't be a concern.

Few screenshots of the integration I'm working on, for context:

Options shown below are for instance applied on a per integration instance basis (via the entities registry).

I hope I didn't go too off-topic nor did wrongful statements... You probably can find more direct information from their documentation, but it'll likely be somewhat time consuming to digest.

Lastly, doing the pull request against the correct branch is the very least I could do! I simply RTFM.
In turn, thanks a lot for dedicating some of your time to this project.

@guymcswain
Copy link
Collaborator

This is a great response - it shows me that you've done your homework. The deal is you will have to own any issues that might arise. That said I plan to merge this PR.

Please take a look at how the documentation is handled. Files that end in .1 for example are autogenerated.

@kurisuD
Copy link
Author

kurisuD commented Jan 27, 2023

Hello Guy

Great, thanks.

Also thanks for the link regarding the documentation, I was definitely under the impression that .1 / .3 files were indeed the final format for man pages and it surprised me to have to do the update - I assume that since they were in the repo (rather than being .gitignored) I had to do the job. I'll go through the link you shared.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants