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

based on machine (pi5) set pwmchip to 2, Also set provider priority … #343

Merged
merged 4 commits into from
Apr 16, 2024
Merged

based on machine (pi5) set pwmchip to 2, Also set provider priority … #343

merged 4 commits into from
Apr 16, 2024

Conversation

taartspi
Copy link
Collaborator

@taartspi taartspi commented Apr 12, 2024

…of Gpiod input/output, linuxFS i2c & pwm, so they are loaded and Pigpio is not. On all legacy Pi (! Pi5) do the opposite priority and pwmchip is 0. I still have the WEB pages updates to do.

@eitch Result, I also had to use the static way to set the Board info for selecting the pwmchip value. Repeated runs showed occasionally the Context was not set in the class, same as I hit with getPriority.

I decided on Pi5 to make the GpioD provider a higher priority then the linuxfs digital. Both higher than Pigpio on Pi5. Below are the providers loaded when all are available

Pi4
---------------------------------------------------------
PI4J PROVIDERS
----------------------------------------------------------
PROVIDERS: [6] "I/O Providers" <com.pi4j.provider.impl.DefaultProviders> 
├─ANALOG_INPUT: [0] <com.pi4j.io.gpio.analog.AnalogInputProvider> 
├─PWM: [1] <com.pi4j.io.pwm.PwmProvider> 
│ └─PROVIDER: "PiGpio PWM Provider" {pigpio-pwm} <com.pi4j.plugin.pigpio.provider.pwm.PiGpioPwmProviderImpl> {com.pi4j.plugin.pigpio.provider.pwm.PiGpioPwmProviderImpl} 
├─DIGITAL_OUTPUT: [1] <com.pi4j.io.gpio.digital.DigitalOutputProvider> 
│ └─PROVIDER: "PiGpio Digital Output (GPIO) Provider" {pigpio-digital-output} <com.pi4j.plugin.pigpio.provider.gpio.digital.PiGpioDigitalOutputProviderImpl> {com.pi4j.plugin.pigpio.provider.gpio.digital.PiGpioDigitalOutputProviderImpl} 
├─SERIAL: [1] <com.pi4j.io.serial.SerialProvider> 
│ └─PROVIDER: "PiGpio Serial Provider" {pigpio-serial} <com.pi4j.plugin.pigpio.provider.serial.PiGpioSerialProviderImpl> {com.pi4j.plugin.pigpio.provider.serial.PiGpioSerialProviderImpl} 
├─ANALOG_OUTPUT: [0] <com.pi4j.io.gpio.analog.AnalogOutputProvider> 
├─SPI: [1] <com.pi4j.io.spi.SpiProvider> 
│ └─PROVIDER: "PiGpio SPI Provider" {pigpio-spi} <com.pi4j.plugin.pigpio.provider.spi.PiGpioSpiProviderImpl> {com.pi4j.plugin.pigpio.provider.spi.PiGpioSpiProviderImpl} 
├─DIGITAL_INPUT: [1] <com.pi4j.io.gpio.digital.DigitalInputProvider> 
│ └─PROVIDER: "PiGpio Digital Input (GPIO) Provider" {pigpio-digital-input} <com.pi4j.plugin.pigpio.provider.gpio.digital.PiGpioDigitalInputProviderImpl> {com.pi4j.plugin.pigpio.provider.gpio.digital.PiGpioDigitalInputProviderImpl} 
└─I2C: [1] <com.pi4j.io.i2c.I2CProvider> 
  └─PROVIDER: "PiGpio I2C Provider" {pigpio-i2c} <com.pi4j.plugin.pigpio.provider.i2c.PiGpioI2CProviderImpl> {com.pi4j.plugin.pigpio.provider.i2c.PiGpioI2CProviderImpl} 
----------------------------------------------------------


Pi5
Pi4J PROVIDERS
----------------------------------------------------------
PROVIDERS: [6] "I/O Providers" <com.pi4j.provider.impl.DefaultProviders> 
├─ANALOG_OUTPUT: [0] <com.pi4j.io.gpio.analog.AnalogOutputProvider> 
├─DIGITAL_OUTPUT: [1] <com.pi4j.io.gpio.digital.DigitalOutputProvider> 
│ └─PROVIDER: "GpioD Digital Output (GPIO) Provider" {gpiod-digital-output} <com.pi4j.plugin.gpiod.provider.gpio.digital.GpioDDigitalOutputProviderImpl> {com.pi4j.plugin.gpiod.provider.gpio.digital.GpioDDigitalOutputProviderImpl} 
├─SPI: [1] <com.pi4j.io.spi.SpiProvider> 
│ └─PROVIDER: "PiGpio SPI Provider" {pigpio-spi} <com.pi4j.plugin.pigpio.provider.spi.PiGpioSpiProviderImpl> {com.pi4j.plugin.pigpio.provider.spi.PiGpioSpiProviderImpl} 
├─PWM: [1] <com.pi4j.io.pwm.PwmProvider> 
│ └─PROVIDER: "LinuxFS PWM Provider" {linuxfs-pwm} <com.pi4j.plugin.linuxfs.provider.pwm.LinuxFsPwmProviderImpl> {com.pi4j.plugin.linuxfs.provider.pwm.LinuxFsPwmProviderImpl} 
├─ANALOG_INPUT: [0] <com.pi4j.io.gpio.analog.AnalogInputProvider> 
├─DIGITAL_INPUT: [1] <com.pi4j.io.gpio.digital.DigitalInputProvider> 
│ └─PROVIDER: "GpioD Digital Input (GPIO) Provider" {gpiod-digital-input} <com.pi4j.plugin.gpiod.provider.gpio.digital.GpioDDigitalInputProviderImpl> {com.pi4j.plugin.gpiod.provider.gpio.digital.GpioDDigitalInputProviderImpl} 
├─SERIAL: [1] <com.pi4j.io.serial.SerialProvider> 
│ └─PROVIDER: "PiGpio Serial Provider" {pigpio-serial} <com.pi4j.plugin.pigpio.provider.serial.PiGpioSerialProviderImpl> {com.pi4j.plugin.pigpio.provider.serial.PiGpioSerialProviderImpl} 
└─I2C: [1] <com.pi4j.io.i2c.I2CProvider> 
  └─PROVIDER: "LinuxFS I2C Provider" {linuxfs-i2c} <com.pi4j.plugin.linuxfs.provider.i2c.LinuxFsI2CProviderImpl> {com.pi4j.plugin.linuxfs.provider.i2c.LinuxFsI2CProviderImpl} 

…f Gpiod input/output, linuxFS i2c & pwm, so they are loaded and Pigpio is not. On all legacy Pi (! Pi5) do the opposite priority and pwmchip is 0
@FDelporte FDelporte requested a review from eitch April 12, 2024 05:59
Copy link
Member

@eitch eitch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You seem to be a C programmer by heart, using the int rval = 0; at the beginning of the method. I think this can be changed to a simple;

BoardModel model = BoardModelDetection.current().getBoardModel();
if (model.getSoc() != Soc.BCM2712) {
  return x;
} else {
  return y;
}

But my bigger question is: Should we really have a different priority for the pigpio provider? What would the benefit of this be? Currently we know users will have a better experience with the Linux FS provider on a Raspberry Pi 5, as the Pigpio doesn't work. But if the user is on a Raspberry Pi 4, then it just depends which dependencies are added to their project.

We currently have three providers for PWM: raspberry, pigpio and linuxfs. The first is basically a stub, the latter the only that works on the Raspberry Pi 5. So in my opinion, that would be a reason to change the priority based on the Board type.

@FDelporte what is your opinion?

@eitch
Copy link
Member

eitch commented Apr 12, 2024

Furthermore, the model.getSoc() != Soc.BCM2712 is a bit cryptic, don't we have a convenience method isPi5()?

@FDelporte
Copy link
Member

Furthermore, the model.getSoc() != Soc.BCM2712 is a bit cryptic, don't we have a convenience method isPi5()?

Good idea, I will add that to the BoardDetection

@FDelporte
Copy link
Member

But if the user is on a Raspberry Pi 4, then it just depends which dependencies are added to their project.

From the tickets it's clear that adding the right dependencies is a problem for a lot of users. So I think the extra priority decision in the plugin can help to tell users to "include all dependencies" and let Pi4J decide which will be used.

@taartspi
Copy link
Collaborator Author

The linuxfs pwm does work on pi4 but only HW and the address having a different meaning I think the old code had linuxfs i2c higher than pigpio I am not sure what is less trouble for users moving to this code base if they have all providers in their pom file. I thought this made sense but Robert u don't and Frank i am not sure your preference I can agree with any plan. So maybe the way to explain your thoughts is us the pi4 output to say what provider u want for each ioType

@taartspi
Copy link
Collaborator Author

@eitch @FDelporte Forgot to add your names so u got notified

@FDelporte FDelporte self-requested a review April 15, 2024 05:52
@eitch
Copy link
Member

eitch commented Apr 16, 2024

Ok, then lets keep it like this

@eitch eitch merged commit 7e02206 into Pi4J:develop Apr 16, 2024
1 check passed
if(BoardInfoHelper.usesRP1()) {
rval = 150;
}else{
rval = 150;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

value is the same in both if cases?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, in both are the same to match what Robert did for 2.5 So I did the update and did pay attention to the fact both legs were equal.
Fixed

// the gpioD driver should be higher priority when on RP1 chip
int rval = 0;
if(BoardInfoHelper.usesRP1()) {
rval = 150;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idem

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be the same as above.
fixed

// the linux FS driver should be higher priority when on RP1 chip
int rval = 0;
if(BoardInfoHelper.usesRP1()) {
rval = 150;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idem

// the pigpio SPI driver should be used over the default
return 100;
// the Pigpio driver should be higher priority when NOT on RP1 chip.
int rval = 0;
Copy link
Member

@FDelporte FDelporte Apr 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be simplified as

        int rval = 100;
        if (BoardInfoHelper.usesRP1()) {
            rval = 50;
        }

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.

3 participants