Pi4J / pi4j-v2

Pi4J Version 2.0
Apache License 2.0
269 stars 56 forks source link

Mock providers don't seem to be loaded on an non-RPi system #368

Open FDelporte opened 1 month ago

FDelporte commented 1 month ago

Related to https://github.com/Pi4J/pi4j-kotlin/issues/12

When initializing a new context on a non-RPi system, it seems the mock plugin is not loaded. Could this be related to the new priority system for plugins, which was introduced recently?

Fix in code is available like this, as used e.g. in unit tests: https://github.com/Pi4J/pi4j-kotlin/blob/master/lib/src/test/kotlin/com/pi4j/ktx/ContextTest.kt

    @BeforeTest
    fun setup() {
        context = Pi4J.newContextBuilder()
            .add(MockPwmProvider.newInstance())
            .build();
    }

I think with the right approach, we could prevent the need to manually add the mock for testing on non-RPi-systems, so this code in the Spring Boot Starter would become unnecessary: https://github.com/Pi4J/pi4j-springboot/blob/main/pi4j-spring-boot/src/main/java/com/pi4j/spring/boot/ContextConfiguration.java#L31

taartspi commented 1 month ago

@FDelporte
I believe the change was prior to the 'provider priorities" change.
https://github.com/Pi4J/pi4j-v2/commit/770a0f621b792ca392b37065447ae09d43fc070d
Implements the possible context creation of: pi4j = Pi4J.newAutoContextAllowMocks(); The result was [/DefaultRuntime.java] when a Mock provider is being evaluated for inclusion the config.autoDetectMockPlugins() is checked.

I see the Core tests cases now use this new means to create the context. So that is what I think happened, you asked what we should do.

  1. document this fact. ( I didn't look to see what the WEB might already say) . But I imagine we cansay more
  2. When we determine the Board info, if not a Pi, load the Mocks under the covers

Since @eitch added a specific means to load the Mocks I want his opinion. I think under the covers is not the way as one, I dislike magic happening unknown to me, and two, what about other providers that may successfully load whether ours or some user implemented providers. Should they be permitted ? Seems a slippery slope.

eitch commented 1 month ago

I think this can be solved by changing this line: com/pi4j/context/impl/DefaultContextBuilder.java:54 from:

protected boolean autoDetectMockPlugins = false;

to

protected boolean autoDetectMockPlugins = !BoardInfoHelper.runningOnRaspberryPi();

What do you think?

eitch commented 1 month ago

See https://github.com/Pi4J/pi4j-v2/pull/373

taartspi commented 1 month ago

I think that is a good solution. Let me know if u want me to assist in further web doc reviews for this work. As it does change behavior you earlier asked about new releases or a point for the ongoing changes. I think this change is a new release (i see u have referred to next release in some items so that seems the plan)