Pi4J / pi4j-v2

Pi4J Version 2.0
Apache License 2.0
274 stars 60 forks source link

Extend the Platform implementation #17

Closed FDelporte closed 3 years ago

FDelporte commented 4 years ago

Note from a discussion on the state of Pi4J V2

The idea was "Platform" represented the target hardware or embedded device and environment a user would run Pi4J from. Such as the "RaspberryPi" platform. A platform could support multiple providers. A platform may assign default providers for each IO type.

When creating a Pi4J Context only a single (default) Platform is held by the context. This platform may be auto-detected based on the presence of a Pi4J platform JAR in the path. If multiple platform JARs are detected there must be some order of precedence determined which is the Default platform.

Not sure how evolved or how complete the concept of Platform is in the current codebase. So it may need some work.

FDelporte commented 3 years ago

Question by Pascal Mathis while testing on CrowPi

Here is a short code snippet that highlights the point I'm talking about:

package ch.fhnw.crowpi;

import java.util.concurrent.TimeUnit;

import com.pi4j.Pi4J;
import com.pi4j.io.gpio.digital.DigitalOutput;
import com.pi4j.io.gpio.digital.DigitalState;

public class App {
    public static void main(String[] args) {
        var pi4j = Pi4J.newAutoContext();
        pi4j.platforms().describe().print(System.out);

        var buzzerConfig = DigitalOutput.newConfigBuilder(pi4j).id("BCM18").name("Buzzer").address(18)
                .shutdown(DigitalState.LOW).initial(DigitalState.LOW).provider("pigpio-digital-output");

        var output1 = pi4j.dout().create(18);
        output1.config().shutdownState(DigitalState.LOW);
        output1.addListener(System.out::println);
        output1.pulse(100, TimeUnit.MILLISECONDS, DigitalState.HIGH);
        System.out.println(output1.provider());

        var output2 = pi4j.create(buzzerConfig);
        output2.addListener(System.out::println);
        output2.pulse(100, TimeUnit.MILLISECONDS, DigitalState.HIGH);
        System.out.println(output2.provider());

        pi4j.shutdown();
    }
}

The output1 GPIO pin created by pi4j.dout().create(18) never seems to work (there is no output on the CrowPi at all), while output2 created using the DigitalOutputConfigBuilder works flawlessly. I noticed that output1 is an instance of com.pi4j.plugin.raspberrypi.provider.gpio.digital.RpiDigitalOutputProviderImpl where as output2 is an instance of com.pi4j.plugin.pigpio.provider.gpio.digital.PiGpioDigitalOutputProviderImpl. When not explicitly stating the provider for the config builder, the same provider as for output1 is used and it also does not work anymore.

Is it actually desired that the pipgio-digital-output provider has to be explicitly specified for each GPIO input/output or should the RpiDigitalOutputProviderImpl somehow figure this out by itself? It currently seems like the latter does not do anything except calling the usual event listeners, e.g. printing the digital change event to stdout as in my example.

ppmathis commented 3 years ago

I've managed to find a workaround for this issue to avoid having to implicitly specify the provider every single time, which greatly simplifies testing our component classes as we do not have to explicitly pass either pigpio-* or mock-*. Basically I've ditched the Pi4J.newAutoContext() and replaced it with this code:

this.pi4j = Pi4J.newContextBuilder()
    .noAutoDetectPlatforms()
    .addPlatform(new CrowPiPlatform())
    .autoDetectProviders()
    .build();

Additionally, the CrowPiPlatform class overrides the existing RaspberryPiPlatform by zeroing out the list of available providers for this platform:

package ch.fhnw.crowpi.helpers;

import com.pi4j.plugin.raspberrypi.platform.RaspberryPiPlatform;

public class CrowPiPlatform extends RaspberryPiPlatform {
    @Override
    protected String[] getProviders() {
        return new String[]{};
    }
}

This is not pretty, but avoids that Pi4J loads the Raspberry Pi providers, causing the PiGpio providers being registered as the default providers. This can also be seen by e.g. comparing pi4j.getPwmProvider().toString():

I did not manage to find any other way to tell Pi4J to prefer the PiGpio providers over the RaspberryPi providers, so this seems like the easiest approach so far.

savageautomate commented 3 years ago

@ppmathis

Yes, the platform auto-detection and provider assignment is very much incomplete.

Here is the theory of what it was intended to do ...

A user could have one or more Platform JARs along with the various Provider JARs in the class-path of a given runtime environment. In the case of an Auto-Context created, the Pi4J runtime context should iterate over all the Provider JARs it finds and interrogate them to see if they should be considered active for the runtime environment and potentially provide a weighting score for each provider. Once the Pi4J runtime context has iterated over all the detected providers and obtains which are valid and their weighted scores it should attempt to initialize with the highest weighted platform and initialize all the providers defined by that platform implementation.

So in the case of CrowPi, CrowPi could certainly have a provider implementation that ranks higher than RaspberryPi if it can detect something in the runtime environment or via hardware that definitively asserts that it is running on CrowPi hardware. However if it does not offer additional hardware I/O or prefer different I/O providers than the default RaspberryPi platform, then there would be no need for it. But I get why you created it for the time being.

The immediate issue you are having is likely due to that no real I/O providers have been defined for the RaspberryPi platform. It looks like there are some stub implementation classes, but no real implementation. I think these were never originally assigned to PiGPIO because we intended to implement LinuxFS GPIO, SPI, I2C and use those as the default providers for the Raspberry Pi platform.

ppmathis commented 3 years ago

@savageautomate Thank you for your detailed explanation about the platform / provider implementation!

However if it does not offer additional hardware I/O or prefer different I/O providers than the default RaspberryPi platform, then there would be no need for it. But I get why you created it for the time being.

You are right, as far as I can tell there would be absolutely no need for a CrowPi specific platform but I was just using that name as it fit into our code base. Once there is no longer any need to use that workaround, the RaspberryPi platform should do just fine.

The immediate issue you are having is likely due to that no real I/O providers have been defined for the RaspberryPi platform. It looks like there are some stub implementation classes, but no real implementation. I think these were never originally assigned to PiGPIO because we intended to implement LinuxFS GPIO, SPI, I2C and use those as the default providers for the Raspberry Pi platform.

This sounds exactly like the issue I am facing, as I also noticed that the RaspberryPi platform does not seem to have any actual I/O providers included, but still registers them, causing them to end up as the non-working default providers despite PiGPIO being available. As the workaround does the job for now though, this is not a blocker in any way - it was mostly just a bit confusing at first why things did not work, but I understand that this is a work in progress.

savageautomate commented 3 years ago

This sounds exactly like the issue I am facing, as I also noticed that the RaspberryPi platform does not seem to have any actual I/O providers included, but still registers them, causing them to end up as the non-working default providers despite PiGPIO being available. As the workaround does the job for now though, this is not a blocker in any way - it was mostly just a bit confusing at first why things did not work, but I understand that this is a work in progress.

Yes, if I recall correctly, I think I was planning on defining RaspberryPi specific provider classes but simply inheriting each implementation from an actual I/O provider such as LinuxFS providers.
https://github.com/Pi4J/pi4j-v2/tree/main/plugins/pi4j-plugin-raspberrypi/src/main/java/com/pi4j/plugin/raspberrypi/provider This way if there are special or unique cases for the Raspberry Pi versions of each provider we have a place to implement these modifications/overrides.

We could just inherit the PiGPIO providers for the time being, but I suspect these ultimately won't be the defaults for the RPi.

FDelporte commented 3 years ago

Has been discussed together with @eitch and @savageautomate and current implementation is considered as sufficient for now until specific use-cases would require more work.

Value weight has been refactored to priority in PR https://github.com/Pi4J/pi4j-v2/pull/95