Pi4J / pi4j-v2

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

Sensor provider and interface #197

Open taartspi opened 2 years ago

taartspi commented 2 years ago

Within pi4j-core I created a new IO type 'sensor' I created the config and provider classes. In my own repo I created a BMP280 sensor using this new IO type. It includes an application example and write up on connecting the BMP280 to a Pi. If you will grant access I will do a pull request with my core changes and point you to the BMP280 implementation for your comments. thanks Tom

FDelporte commented 2 years ago

Hey @taartspi, thanks for your work! Are you working in a fork of this project? In that case you can create a pull request from your project to this one.

taartspi commented 2 years ago

Morning. No I used the url and of the core repo. But I will recreate a fork and move the parts into that repo. Thanks sir

taartspi commented 2 years ago

I forked the pi4j V2 and added my Sensor code. That has now been pushed and I created a pull request for your viewing. And these changes can be tested using my repo https://github.com/taartspi/Pi4J_V2-TemperatureSensor Once we agree on what I was implementing in the core, I would like review/comments on the BMP280 work whether it could stand as a simple example of device implementation. @FDelporte

taartspi commented 2 years ago

@FDelporte Hello Frank. Is there an expected timeframe when my pull request would be considered ? I don't know how busy all of you are, but I prefer the pull not exceed 30 days and be deleted. As I said in the details if the pull was accepted after review/updates, there is a BMP280 implementation I would like considered as a simple device example. thanks Hope you are well. Tom

FDelporte commented 2 years ago

sorry, for delay, too few hours in a day, will take a look ;-)

taartspi commented 2 years ago

@FDelporte @savageautomate @eitch The changes to the core is the addition of a Sensor interface and Sensor providers. The provider assumes the sensor needs a bus and address but does not assume the state pertains to i2c or spi or any other interface.

In my own repo i used these to implement the code specific to a BMP280

taartspi commented 2 years ago

pull #199 Long ago we talked about a generic Sensor interface, this is a shot at creating that and its Config and Provider classes.

Then in my own repo I did an implementation specific to BMP280 using these core changes. If this is something you still want to discuss let me know, if no longer of interest I will delete the request to clear it off the books

Tom

FDelporte commented 2 years ago

I see the use-case for a generic sensor interface. So these methods indeed fit in this approach

resetSensor
initSensor

What doesn't really "fit" into the generic approach of the V2 library, or specific methods like the following as they are coupled to a specific use-case and type of sensor.

temperatureC()
temperatureF()
pressurePa() 
pressureIn() 
pressureMb()

I'm not sure yet how this could be made more generic, but thinking more of an approach with e.g.

value(SensorType type)

Where SensorType could be an enum defined by the implementation? So in case of a Temp sensor

SensorType.TEMP_C
SensorType.TEMP_F

Just my 2 cents...

taartspi commented 2 years ago

Good day Frank @FDelporte I understand that you prefer no config/provider class as they are still too specific in thought and only handles two sensor types, preferring only the interfaces are defined. Thinking of more sensor types I agree my implementation is not appropriate. Did a little digging on your idea of an enum defining the interfaces and need to spend more time looking/learning on that idea. Let me give this some thought as the idea of sensor quickly explodes to: temp, humidity, texture, pressure, color, salinity, clarity, etc. Lets see what I create so we look at it for more discussion on whether this is of value or too cumbersome to be useful.

thanks Tom.

PS I will delete the existing PULL.

FDelporte commented 2 years ago

Additional feedback regarding the addition of sensors

(1) <> Originally Sensors, Switches, Relays, Lights, LEDs, Servos, etc were part of the Pi4J V1 device/components library (https://github.com/Pi4J/pi4j-v1/tree/release/1.3/pi4j-device/src/main/java/com/pi4j/component). They never received much use and thus not much attention was focused on building them out further. With the launch of V2 I was opposed to including them mainly due to bloat and recognizing the fact that I could not devote the time necessary to properly manage and maintain them and I wanted Pi4J V2 to strictly focus on the hardware layer I/O. With that said, I certainly see the desire for Java-based generic higher level component interfaces. (which is why they existed in Pi4J V1) If the Pi4J project wants to support these again, I think the question that remains is should they be placed in the core library or should a separate Pi4J-Components auxiliary (and optional) library be created to house these? I’m not weighing in with an opinion, just framing the issue/question.

(2) <> If the project decides to add support for component layer interfaces like Sensors, I would suggest some interface design work going into the interfaces to ensure they each have a consistent API spec. For example, I would create a general Sensor marker or base interface and then have component specific interfaces like TemperatureSensor, PressureSensor, HumiditySensor, etc that implement the generic/base Sensor interface. There are probably some valuable tidbits to glean from the older implementation as well so it may be worth reviewing it: See https://github.com/Pi4J/pi4j-v1/tree/release/1.3/pi4j-device/src/main/java/com/pi4j/component/sensor and https://github.com/Pi4J/pi4j-v1/tree/release/1.3/pi4j-device/src/main/java/com/pi4j/component/temperature

taartspi commented 2 years ago

This discussion is also contained in 138 and 151. Having beat this horse several ways I agree any implementations should be separate from the Core. At the present time there is the crowpi project and the seven devices I implemented to provide an easy way for a person to attach a device and try Pi4J_V2. They also provide different examples of coding in V2. Any discussion of adding a separate component layer would take considerable time from an already busy team. I looked at the V1 code you identified and will put some cycles thinking about what is appropriate in V2. When I have any suggestion I will code it up and make it visible to you to get an opinion on the direction.