dotnet / iot

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

GPIO input gives segmentation fault on BeagleBone Black in v3.1.0 #2313

Closed FredMurphy closed 3 weeks ago

FredMurphy commented 1 month ago

Describe the bug

Opening a pin for input on a BeagleBone Black gives a Segmentation Fault in v3.1.0. The same code works in v3.0.0

Steps to reproduce

using System.Device.Gpio;

GpioController controller = new();
GpioPin buttonPin = controller.OpenPin(67, PinMode.Input);

code compiled with the following and then copied over via SCP:

dotnet publish --runtime linux-arm --self-contained

Expected behavior An input pin should be opened and value can later be read.

Actual behavior Segmentation fault as the pin is opened. Output pins work fine.

Versions used dotnet version 8.0.204 (also tried 6.0.421) BeagleBone Debian 11.7 minimal (although seems to happen on other versions too) System.Device.Gpio version 3.1.0

Reverting System.Device.Gpio to 3.0.0. fixes the issue.

pgrawehr commented 1 month ago

Can you add the result of GpioController.QueryComponentInformation to the question? I don't remember any relevant changes there.

FredMurphy commented 1 month ago

3.0.0 (working)

{
  "Name": "System.Device.Gpio.GpioController",
  "Description": "Generic GPIO Controller",
  "Properties": {
    "OpenPins": ""
  },
  "SubComponents": [
    {
      "Name": "System.Device.Gpio.Drivers.SysFsDriver",
      "Description": "Gpio Driver",
      "Properties": {},
      "SubComponents": []
    }
  ]
}

3.1.0 (not working)

{
  "Name": "System.Device.Gpio.GpioController",
  "Description": "Generic GPIO Controller",
  "Properties": {
    "OpenPins": ""
  },
  "SubComponents": [
    {
      "Name": "System.Device.Gpio.Drivers.LibGpiodDriver",
      "Description": "LibGpiodDriver",
      "Properties": {
        "LibGpiodVersion": "1.6.2"
      },
      "SubComponents": []
    }
  ]
}

No surprises that this fixes the problem when running 3.1.0, but obviously the default for the BBB is not ideal.

GpioController controller = new(PinNumberingScheme.Logical, new SysFsDriver());
pgrawehr commented 1 month ago

At least there's a workaround ;-)

I guess we have to analyze why

a) 3.0.0 prefers sysfs over libgpiod and b) libgpiod doesn't work

Can you verify whether libgpiod would work with 3.0?

FredMurphy commented 1 month ago

I hadn't thought to check that. 3.0.0 also gives a segmentation fault when asked to use LibGpiodDriver,

pgrawehr commented 1 month ago

[Triage] This somehow sounds like a problem in libgpiod itself. There's a command line tool called gpiod which you could use to check whether the bug is in the libgpiod library itself or in our code. Here's a link: https://lloydrochester.com/post/hardware/libgpiod-input-rpi/

First try to run gpiodetect to make sure you are using a valid controller and line.

FredMurphy commented 1 month ago

gpiod seems to work fine on its own. My above example logical pin of 67 maps to gpiod chip2 pin3. The following works and gives a 0 or 1 depending on whether I connect the pin to GND or 3V3.

gpioget gpiochip2 3
raffaeler commented 1 month ago

gpiod seems to work fine on its own. My above example logical pin of 67 maps to gpiod chip2 pin3. The following works and gives a 0 or 1 depending on whether I connect the pin to GND or 3V3.

gpioget gpiochip2 3

The gpiochip number is most probably the culprit. In fact, by default the libgpiod driver in this repository defaults to zero.

Could you please try creating the libgpio driver passing 2 to the constructor? https://github.com/dotnet/iot/blob/main/src/System.Device.Gpio/System/Device/Gpio/Drivers/Libgpiod/LibGpiodDriver.cs#L24

Then you pass this instance to the GpioController (the default PinNumberingScheme is PinNumberingScheme.Logical) https://github.com/dotnet/iot/blob/main/src/System.Device.Gpio/System/Device/Gpio/GpioController.cs

If this still does not work, there may be a mismatch from the logical/physical pin number due to the PinNumberingScheme. You can try changing it. There may still be the need to create a mapping specific to this BeagleBon board.

Thanks

FredMurphy commented 1 month ago

This works - using gpiod and referring to the pin as gpiochip2 pin 3, rather than the pin 67 that worked when 3.0.0 defaulted to SysFsDriver:

GpioController controller = new(PinNumberingScheme.Logical, new LibGpiodDriver(2));
GpioPin buttonPin = controller.OpenPin(3, PinMode.Input);

As we ascertained earlier, this also works:

GpioController controller = new(PinNumberingScheme.Logical, new SysFsDriver());
GpioPin buttonPin = controller.OpenPin(67, PinMode.Input);

I suppose we can put this down to a change in the default driver from SysFsDriver to LibGpiodDriver and the fact that the pin numbering then has to be changed to match. Not great that previously working code breaks at runtime with nothing but a NuGet package update though.

raffaeler commented 1 month ago

This works - using gpiod and referring to the pin as gpiochip2 pin 3, rather than the pin 67 that worked when 3.0.0 defaulted to SysFsDriver:

Great to hear that.

The mismatch between physical and logical pins depends on the absence of a specific driver like this one: https://github.com/dotnet/iot/blob/main/src/devices/Gpio/Drivers/OrangePiZero2Driver.cs#L16

It may also be important to find a way to detect the board and pick the correct driver as it happens here: https://github.com/dotnet/iot/blob/main/src/System.Device.Gpio/System/Device/Gpio/GpioController.cs#L470

@pgrawehr Am I forgetting anything else?

@FredMurphy Would you be interested in contributing to supporting the BeagleBone Black with a pull-request?

Thanks

FredMurphy commented 1 month ago

@FredMurphy Would you be interested in contributing to supporting the BeagleBone Black with a pull-request? I'd be happy to. It might take me a little while to get my head round how things are structured and fit it in with work commitments, etc. though.

raffaeler commented 1 month ago

That would be great! None of us in the Triage team has the board, but feel free to ask any doubts. You can start looking at the existing drivers to figure out the general structure.

Thanks

joperezr commented 1 month ago

[Triage]: We are trying to figure out what is the root cause of the segfault. Is it because when we PInvoke we do it with a chip that is wrong? Or is it because we are not catching an error returned by that PInvoke and trying to use the handle with the invalid handle? @raffaeler or @pgrawehr will try this out in the following week. If the issue is the former, we'll have to investigate and see how to fix but we won't block the release for this. If the issue is the latter, then we will fix the catching the error before our next release.

FredMurphy commented 1 month ago

I'm happy to do some testing if nobody has a BeagleBone Black, I'm guessing that the changed default driver meant I was unwittingly trying to use the LibGpiodDriver with chip 0 and pin 67

joperezr commented 1 month ago

That would be super helpful, yes. You should be able to create the Driver yourself var driver = new LibGpiodDriver(2) to specify the chip 2, and then pass that in to the GpioController. If that works, then we'll confirm what the root cause is. From there, it would be good what is throwing the segfault, and check if we could catch some error in order to "fail in a better way" meaning with a better message.

raffaeler commented 1 month ago

From the previous messages I understand that @FredMurphy already successfully tested that.

The only reason now to create a specific driver is to bake the pin mappings in it, which afaik requires talking to different gpio chips depending on the gpio being accessed.

@FredMurphy Could you please confirm or correct my statements?

raffaeler commented 3 weeks ago

I confirm that the Segmentation Fault error always occurs when the gpiochip is wrong. I tested this on a RPi4 with Raspbian/Bullseye 32-bit fully updated by specifying a gpioChip different than 0.

raffaeler commented 3 weeks ago

The segmentation fault is definitely a bug in the Libgpiod driver implementation: image

As you can see the SafeHandle is not null but the wrapped handle is invalid which ultimately causes the fault.

raffaeler commented 3 weeks ago

@FredMurphy This issue was closed as the segmentation fault is now fixed by my PR.

With regards to the BeagleBone support, since I understand you are interested in contributing with a PR, you can either directly open the PR when you are ready or even create a new issue in case you want to discuss the topic before the PR.

Thank you

FredMurphy commented 3 weeks ago

With regards to the BeagleBone support, since I understand you are interested in contributing with a PR, you can either directly open the PR when you are ready or even create a new issue in case you want to discuss the topic before the PR. Perfect.

I agree that BeagleBone improvements are distinct from this bugfix. I'm working on it now, but work commitments may mean that a PR is ready soon... or not so soon.

raffaeler commented 3 weeks ago

Great, take it easy, no pressure of course, it's a (very welcome) contribution :-)