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

Support for other boards #15

Closed
SecT0uch opened this issue Nov 4, 2020 · 59 comments
Closed

Support for other boards #15

SecT0uch opened this issue Nov 4, 2020 · 59 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@SecT0uch
Copy link
Contributor

SecT0uch commented Nov 4, 2020

Would you consider support for other boards ?

I have a setup with a Pine64, started developping something similar and came across your project (which is awesome btw).

One blocking problem is the dependency RPi.GPIO, which works with Raspberry Pi only.
One possible solution would be to migrate to python-periphery.

I'm willing to contribute to the project in a separate branch to go in that direction if you're OK with that.

@yeyeto2788
Copy link
Contributor

Another way to use other boards would be using Adafruit's Blinka which will enable you to use other Linux based boards. I actually added the support for some Pine64 devices on that and they are working as expected.

If there is help needed I can also provide some help.

@olixr
Copy link
Member

olixr commented Nov 4, 2020

Support for more boards is welcome! I would go down the route of an adapter then for the GPIO and have this toggled in configs based on the device. This would allow users to easily swap out GPIO for other libraries and add support easier.

We should extract any calls directly to the GPIO into a GPIO adapter. This adapter should have a uniform syntax that can be used in MudPi without needing to change based on on the library you choose to use. I am going down a similar path with some of the event emitters.

Please start a branch and submit some items as you like. I would be happy to review and see what we can do. I personally do not have those boards to test on so I would need some help verifying changes later on.

Thanks for taking this on!

@olixr olixr added enhancement New feature or request question Further information is requested labels Nov 4, 2020
@SecT0uch
Copy link
Contributor Author

SecT0uch commented Nov 4, 2020

@yeyeto2788 I've actually never used blinka.
From my understanding, it allows us to run CircuitPython code on a classic CPython.

What would be the advantages of using CircuitPython/blinka layer than directly using python-periphery or the libgpiod Python wrapper ?

Edit: I think I got your point, it would allow us to use the Adafruit libraries (sensors, etc.), which would be useful.

@olixr Just found that https://github.com/underground-software/RPi.GPIO2, which would be a simpler solution and would allow us to keep the same syntax. Never tested though

@yeyeto2788
Copy link
Contributor

Hey @SecT0uch,

IMHO the easy part is that it automatically detects GPIO pins and interfaces on the SBC automatically and creates the object instead of you writing the file path manually of the GPIO/interface as you would do on python-periphery (based on my poor experience with it).

In the end, CircuitPython is a port of MicroPython which is a re-implementation of Python for Microcontrollers.

Blinka lets you write Python code to interact with many Linux SBC so let's say you want to toggle an led is as simple as doing:

import board
import digitalio
import time

led = digitalio.DigitalInOut(board.A1)
led.direction = digitalio.Direction.OUTPUT

while True:
  led.value = False
  time.sleep(1)
  led.value = True
  time.sleep(1)

Also the good think about it is the broad range of sensors that can be used for which Adafruit has already worked on.

Maybe you can share your thoughts on python-periphery and I would change my mind 😛

@SecT0uch
Copy link
Contributor Author

SecT0uch commented Nov 5, 2020

That's what I figured out. :)

I'm thinking of a way to completely replace RPi.GPIO by blinka, without changing the configuration too much.
It would work with Raspberry Pi + all the other (blinka-compatible) boards.

We should target:

  • Raspian boards : Raspberry Pi
  • Armbian boards: The others

Reference for pin mapping

The advantages:

  • More compatible boards
  • Wider public (and potentially contibutors)
  • Replaces deprecated libraries : RPi.GPIO, Adafruit_DHT
  • Using an unified solution

I know it would need some refactoring, but really think it would worth the effort.

Any thoughts ?

@yeyeto2788
Copy link
Contributor

I completely agree with that, I can test on some boards I have laying around.

The only comment I have is that not all armbian boards are supported by blinka but for example, I know for sure that Pine64 devices are supported and the majority of the Orange Pi boards.

@SecT0uch
Copy link
Contributor Author

SecT0uch commented Nov 5, 2020

Actually working sensor.py and humidity_sensor.py to make it work with blinka.
I'm testing on my Raspberry Pi Zero W.

I also have a SoPine64, but will test that when everything is fully working on Raspis.

I'll come back later to share my work-in-progress.

@olixr
Copy link
Member

olixr commented Nov 5, 2020

I think support for multiple boards would be great. I have used the blinka a bit but not extensively. Once you get a few tests going the first big step will be to go and extract all the current GPIO calls into a GPIO Adapter. Using adapters will make it easy to swap out the library without changing the core calls once completed.

Each GPIO adapter will follow expose the exact same methods while keeping the underlying implementation details specific to the board. I.e. we should just call adapter.digitalRead() and behind the scenes it would make a call to the libraries to actually take the digital reading.

I expect the adapter to be relatively simple in its api so the main work will be the initial extraction and then it would be easier to add adapters going forward. This is the strategy I will be applying to other areas of the system too like the event system. Using adapters it will be easier to provide multiple support options without a complex web of calls based on configs.

We can introduce a gpio config object to the configs that can contain a platform to specify the gpio platform desired.

Here would be an example of pi gpio

'gpio': {
    'platform': 'gpio' 
}

Then we can make adapters and options for pine64, blinka, and others.

@SecT0uch
Copy link
Contributor Author

SecT0uch commented Nov 5, 2020

I'm not sure to understand how or why we would need adaptors as migrating to Blinka would make it compatible with Pi + all the blinka compatible boards, without having to specify the board.

I started going down that path:

  • Created sensors/linux and workers/linux
  • Copied and adapted sensor.py, humidity_sensor.py, worker.py and sensor_worker.py to replace RPi dependancies with blinka.

I have a working example with my Pi zero and a DHT22:
See my WIP branch
I'm using the following config:

{
    "name": "NewBoard SecT0uch",
    "version": 0.8,
    "debug": false,
    "redis": {
        "host": "127.0.0.1",
        "port": 6739
    },
    "relays": [],
    "workers": [
        {
            "type": "sensor",
            "sensors": [
                {
                    "pin": "D12",
                    "is_digital": true,
                    "model": "22",
                    "type": "Humidity",
                    "key": "HumTem_sensor",
                    "name": "MyTempHumSensor"
                }
            ]
        }
    ],
    "nodes": []
}

With this configuration and my changes, I'm successfully reading the Humidity and Temperature using blinka libraries.

The only configuration difference is pin value.
For example, for a Raspberry pi instead of setting "pin": "12", it would be "pin": "D12". D for digital, A for analog
Each user can refer to his board pins here
For example, you can find the Raspberry pi pin names here

To make it work with a Pine64 or whatever, I just need to change the pin value and that's all.

Once the migration completely done, we could rename the pi folders to linux and all the linux board would use it (incl. Raspi).
Does it make sense ?

If you allow me, I can continue going down that path to get rid of RPi dependencies, without losing Raspberry Pi compatibility.

Side notes:

  • In all the files I edited, I replaces the tabs with 4 spaces to make it PEP8 compliant. (You can configure your IDE to automatically insert 4 spaces when you press )
  • Is it really necessary to do sys.path.append('..') before importing your modules ?
  • I noticed that in most of the files there are imported modules but unused. Ex: time and json in sensors/pi/sensor.py. Are they actually used ? Can we remove them safely ?
  • I had to specify the key and model value manually because the config builder is not generating it . Did I miss something ?

@olixr
Copy link
Member

olixr commented Nov 5, 2020

The main reason I suggest adapters was to be open for future additions. I personally have not had any other boards to even test this on though. For the time the path you have started on should be fine as it still supports the pi while gaining additional board support. The blinka uses RPi GPIO under hood for the pi anyway so it just adds more support for those other linux boards while keeping same pi behavior.

The breaking change in pin configurations would be acceptable given the features we are adding on. Changing the folder names makes sense too.

I had tabs in my files coming from PHP and following PSR a lot. Didn't spend a lot of time digging into the python standards on this yet so open to going with PEP8 or whichever is the common one.

The sys.path.append('..') is linked to the dynamic imports if I remember. This and a combination of me stubbing out new features while not removing some debug items i.e. additional module as you see. I am currently working on another large refactor to consolidate a lot of things and bring more cohesion to some underlying systems. In the refactor I will go do some trimming on modules and left over stuff from before.

The config tool on the site is v0.8 and why you are noticing this difference. The core had been updated a lot recently and I haven't got around to update the config tool. It will be updated after I finish some more of these large core changes. I am only a solo developer on this project so it takes me some time to get around to all the various odds and ends. Plus I have a few week old newborn at the moment :)

Keep up the good work I am looking forward to the updates. I have some large updates I am doing as well so I will want to merge this in soon that later to my branch.

@SecT0uch
Copy link
Contributor Author

SecT0uch commented Nov 5, 2020

Yeah sorry, I had no clue on how to implement adapters. But good to know I can keep going.
I'll leave the unused modules and let you see what you want to do with that later on.

Nothing urgent, I know what it is to be a solo dev. Take care of you family :)

I will submit a PR for you to check when I'm done.

Thanks

@yeyeto2788
Copy link
Contributor

@SecT0uch if you finished your part I can handle some refactor and some linting tomorrow based on your branch so I can file a PR against your branch and then you can file a PR to @olixr branch so it gets easier for him to merge everything at once.

@olixr it would actually recommend you to wait a bit more for your changes so you don't stumble across many merge issues.

Does ot sound good to guys?

@SecT0uch
Copy link
Contributor Author

SecT0uch commented Nov 6, 2020

@yeyeto2788 Fantastic!
Didn't finish yet, I'm done for today.
I'll let you know :)

@yeyeto2788
Copy link
Contributor

Hello guys!

I have created the PR https://github.com/SecT0uch/mudpi-core/pull/1, I haven't had the chance to test it but it should be any difference in functionality and it should be working because the changes are:

  • PEP8 naming conventions
  • PEP8 max character 79 (Not all documents are done but most of them)
  • Create a BaseSensor object in order to delete code duplication when initializing any sensor.
  • Add google Docstrings on some method, objects, and functions.

Things I noticed:

  • There are several constants that should be moved to a separate file for consistency.
  • We should about doing the sys.path.append('..') at all levels and use absolute import statements like here
  • Variables should have more explicit names rather than using (r, i, f)

If there is anything else I can help with let me know, also I would try to test it tonight.

@SecT0uch
Copy link
Contributor Author

SecT0uch commented Nov 6, 2020

Hey guys!
Thanks for you PR @yeyeto2788.

I'm having trouble finding an equivalent to GPIO.RISING, GPIO.FALLING, GPIO.BOTH and bounctime= in control.py.

I'm using digitalio and I think adafruit_debouncer or gamepad might help.

Any thoughts ?

I don't even have any push button or switch to test :/

@yeyeto2788
Copy link
Contributor

Shouldn't that be covered by the drive mode?

No need of button when you have cables 😛

@SecT0uch
Copy link
Contributor Author

SecT0uch commented Nov 6, 2020

From my understanding the drive mode allows us to define an output with PUSH_PULL or OPEN_DRAIN mode.
How would that help me for an edge detection ?

I just commited my WIP on that file.

Could you hint me when you have some time?
These lines, this one and that one are concerned.

@olixr
Copy link
Member

olixr commented Nov 6, 2020

I am not sure the equivalent of this in blinka but every time I use the buttons on the Pi edge detection is used. Otherwise you typically get some multi readings as you know. Searching some of the docs on blinka its not clear what they have to do this. Perhaps @yeyeto2788 you know of this library more? I want to be sure we test this well. I have some test builds on the bench here but it will help if you are able to do some initial verification on any changes.

Also the items you both have mentioned during your updates align with much of the planned updates I have. It would help if I share some of the updates as it addresses much of what we are discussing. To give an idea of the direction here are some of the bigger updates:

The constants will be moved out to their own file along with variables. Much of the classes will now have more explicit importing. Removing the need for the path append.

The passing of the temp variables are going to be cleaned up as I introduce the new features like the StateMachine and continue putting in adapters for the new event system. There won't be the need to pass down the redis connection because an event bus will now handle broadcasts on multiple channels as desired globally. This allows me to add MQTT as I am using this in my home now along with the garden.

Workers as well will have new base classes created and cleaning up the inheritance change across the system.

There is a batch of other things on the list as well. I will share my feature list sometime and this will give an idea of the items slated for the core. Along with the core I am working on the UI and installer plus a few other additions. Excited to release some of these soon.

I am looking to merge in what you have and I will continue the more in depth changes. As you noted the sensors and other items have some duplication and extend from a few different areas. I will be some large foundation refactoring making a base Component class that will help restructure all components attached to the system.

@SecT0uch
Copy link
Contributor Author

SecT0uch commented Nov 7, 2020

I'll take my relay card out tomorrow to test the relay worker on my pi.
I also have a T9602 and will look if I can add it to the sensors and test the I2C worker.

I want to make sure everything works well with Raspberry Pi before to start testing on my SOPine.

I am looking forward your updates and refactoring :)

@yeyeto2788
Copy link
Contributor

@SecT0uch for edge detection I could only find this but seems like an old version of circuitpython and from what I've been looking in blinka i didn't see any reference to it. The interesting part is that for Neotrellis the are actually using it as in here

I'll keep looking more info. If I don't find anything I'll open an issue in Blinka.

In the other hand though, if you do not set the edge_detection then it is not used.

If there is anything else I can do let me know, also if you want me to go over the code once again and clean it more let me know.

@olixr what changes are going to perform on UI? Also I think it would be great to have a list of the features/bugs per component (UI, core, etc) you would like to introduce. Maybe creating an issue for each feature/bug on each repo.

@SecT0uch
Copy link
Contributor Author

SecT0uch commented Nov 7, 2020

@yeyeto2788 Yeah I think opening an issue would help us. I tried to ask on their Discord, but no reply.

I did some initial testing with the relay worker and it seems to work fine. I'll do more testing later today.

@olixr I noticed you are in converting the temperature to Fahrenheit before saving it in the redis database.
Shouldn't we save it in Celsuis (what outputs most of the libraries) ?
We could then allow the user to choose his favorite unit via the ui, and then do the conversion if Fahrenheit is chosen. (I didn't check the ui code yet, maybe it's already the case)
It would avoid approximations by converting (and rounding) both ways.

Do you have an example on how declare the bme680 i2c sensor in the config file ?

@olixr
Copy link
Member

olixr commented Nov 7, 2020

That conversion is from an old version like v0.2. My plan is to add config option for METRIC_UNIT or IMPERIAL_UNIT and use this as a constant throughout. This will be done when I introduce the revised constants and variables. Then this would solve any future components as well that give data in different units. For example a ultrasonic range sensor may be in CM or IN based on configs.

I2C config pulled from the docs:

{
    "workers":[{
        "type":"i2c",
        "sensors": [
            {
                "type":"Bme680",
                "address": 119,
                "name":"Barometer",
                "key":"barometer"
            }
        ]
    }]
}

Note that hex isn't supported in json so ints are used in the address that get cast to hex internally.

@SecT0uch
Copy link
Contributor Author

SecT0uch commented Nov 7, 2020

I added the I2C sensor t9602, tested and works well! See here.
Here is the config:

"workers": [
	{
	   "type": "i2c",
	   "sensors": [
                {
                    "address": 40,
                    "type": "T9602",
                    "key": "MyHumSensor2",
                    "name": "MyTempHumSensor2"
                }
	   ]
        }
    ]

There is no adafruit library for this sensor, so I didn't use blinka's busio but smbus (still not Pi-dependent).

@yeyeto2788
Copy link
Contributor

Morning guys!

Sorry for the late reply on the issue creation, you should have received a notification on your inbox with the issue created on Addafruit's Blinka library (Sorry if I take any attribution that doesn't correspond to me). I hope we can get a bit of support.

@SecT0uch
Copy link
Contributor Author

SecT0uch commented Nov 9, 2020

Well done, thanks! :)

@SecT0uch
Copy link
Contributor Author

SecT0uch commented Nov 9, 2020

Hey guys,

The porting is almost finished.
The only Pi specific worker left is camera_worker.py. From the code:

'''
This is (for instance) a Raspberry Pi only worker!

The libcamera project (in development), aims to offer an open source stack for cameras for Linux, ChromeOS and Android.
It will be able to detect and manage all of the exposed camera on the system. Connected via USB or CSI (Rasperry pi camera).
libcamera developers plan to privide Python bindings: https://www.raspberrypi.org/blog/an-open-source-camera-stack-for-raspberry-pi-using-libcamera/#comment-1528789

Not available at time of writing: 9 Nov 2020

Once available, we should look forward migrating to this library, as it would allow our worker to support multiple boards and devices.
'''

There is a detail in which we need to think
Remember the new pin declaration: "pin": "D12"
How could we manage regarding Arduino declared sensors? At the moment it's still the old format "pin": 12, as I didn't touch this part of the code.

What's left to be done:

  • Fully port
    • Sensors
    • I2C
    • Relays
    • Controls
    • Figure out how to declare pins for nodes
    • Implement USB camera
  • Update Documentation
  • Test on Raspberry Pi:
    • DHT sensors
    • Float sensor
    • T9602 I2C sensor
    • BME680 I2C sensor
    • Relays (I did check, but we may want to double check that)
    • Controls (switch and button)
  • Test on an armbian system:
    • DHT sensors
    • Float sensor
    • T9602 I2C sensor
    • BME680 I2C sensor
    • Relays (I did check, but we may want to double check that)
    • Controls (switch and button)
    • LCD
  • Adapt installation script to armbian if needed
  • When libcamera python bindings available, migrate to it

I'm missing hardware for the additional tests.

@olixr
Copy link
Member

olixr commented Nov 10, 2020

Hey guys,

The porting is almost finished.
The only Pi specific worker left is camera_worker.py. From the code:

It will be best to support for both libs at the moment until its fully able to replace pi camera. In the Camera there can be config like USB or something similar that we can use to determine what library to load up. I will look into this for a future refactor and work on that. There will be support for both until the library advances further. Once its a little more updated then I don't mind just using the one.

There is a detail in which we need to think
Remember the new pin declaration: "pin": "D12"
How could we manage regarding Arduino declared sensors? At the moment it's still the old format "pin": 12, as I didn't touch this part of the code.

Since the nodes do not use blinka the config can remain as it is. Also for the Pi / Linux I had configs for is_digital which can now be used in the transition. I will work on some variation of this for legacy support. For example if only a pin number is defined I can use the additional configs to prefix an "A" or "D". This would allow for the new syntax outright and a bridge for old configs as we transition. Similarly this would apply to the nodes and allow us to make similar updates in the future.

Also on the subject of nodes, while I like nanpy the library is not under active development from the original author anymore. In addition I had to add the ESP32 support which still has documented problems with the Wifi on the espressif github. This will likely lead me to add additional 'active-node` options in the future. Something to consider is all.

I'm missing hardware for the additional tests.

I will run a quick suite of tests in the morning on my systems. I am starting to dig into some of my refactors and will be wanting to merge in this update to my branch. The plan is I will merge this into my feature branch and push with the v0.9.2 or v0.9.3 update :)

Keep up the good work. It will be nice to see other linux boards able to run the core now.

@SecT0uch
Copy link
Contributor Author

@olixr, do you validate the following modifications from @yeyeto2788 PR befor I merge it:

  • Removal of unused dependencies
  • Removal of sys.path.append('..')
  • Moving load_config_json to utils.py

@yeyeto2788
Copy link
Contributor

Hello guys!

I'm trying on the OPI lite as I said before, we need to think a bit more the naming convention on pin assignments since on my board the declared pins are the followings:

Python 3.8.5 (default, Jul 28 2020, 12:59:40)
[GCC 9.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import board
>>> board.
board.I2C(      board.PA19      board.PC3       board.SCLK
board.MISO      board.PA2       board.PC4       board.SDA
board.MOSI      board.PA20      board.PC7       board.SPI(
board.MOSI1     board.PA21      board.PD14      board.TX
board.PA0       board.PA3       board.PG6       board.ap_board
board.PA1       board.PA6       board.PG7       board.board_id
board.PA10      board.PA7       board.PG8       board.detector
board.PA11      board.PA8       board.PG9       board.pin
board.PA12      board.PA9       board.RX        board.sys
board.PA13      board.PC0       board.SCK
board.PA14      board.PC1       board.SCK1
board.PA18      board.PC2       board.SCL

I have also modified Adafruit's libgpiod installation script here which is useful for not doing manual steps.

I'll also update the PR with handling errors when there is no mudpi.config file. Not handling this at the beginning will cause also errors all over the application since all components use it (even the logger)

@SecT0uch
Copy link
Contributor Author

SecT0uch commented Nov 10, 2020

Indeed you're right. Each board has specific pin names.
The user needs to find out which of his pins are the GPIO ones.
I will edit the comments to specify it.

For example, for a Raspberry pi instead of setting "pin": "12", it would be "pin": "D12". D for digital, A for analog

This is only true for Raspberry Pi boards.

@olixr
Copy link
Member

olixr commented Nov 10, 2020

For the camera do a try: import like I do with the nanpy and store a const if the import worked. Look at the top of mudpi.py for how I do it. Use this method for now until we find a full drop in replacement for the pi camera.

  • Removal of unused dependencies
  • Removal of sys.path.append('..')
  • Moving load_config_json to utils.py

These changes are fine. The config I plan some bigger work on once I add YAML support. For now this can live in a utils file.

For the pin declarations, we will no longer be able to do casting to int or str since they now have multiple options. I think the documentation will have to outline this well. Without a specific board check it will be more up to the user to provide proper configurations. I suggest we pull out a list of pin declarations and I will include them on the docs so folks don't need to poke around a constants file. Otherwise I don't see that this is anymore we can do but leave it up to them with documentation.

@olixr
Copy link
Member

olixr commented Nov 11, 2020

I am starting some more work on my feature branch. This can be merged into your work as needed until we get a PR in place. I am holding off on bigger refactors but working on the other items that should not conflict to much.

@yeyeto2788
Copy link
Contributor

yeyeto2788 commented Nov 11, 2020

@olixr don't take me wrong but it seems like you didn't merge our changes into your feature branch.

You've created the const.py file which is similar to the constants.py we created on our fork renaming the variables.py so anyone would add any constant into the file. Also, the addition of the server and logs on it are changes that will definitely conflict with @SecT0uch branch.

My humble recommendation would be to merge @SecT0uch branch into yours to avoid further conflicts.

BTW I did a lazy search in order to change documentation but didn't find it, can you please point me to it?

@SecT0uch
Copy link
Contributor Author

Yeah I didn't create the PR yet. @olixr do you want me to create the PR on feature or master branch ?

@olixr
Copy link
Member

olixr commented Nov 11, 2020

@yeyeto2788 I pushed changes I had without merging. Merges still need to take place :)

The documentation/marketing site is not posted publicly anywhere. Its (the docs) all wrote in markdown though so feel free to write up anything in markdown and send it to me for updates.

@SecT0uch Lets plan to merge into feature branch. This will allow me to keep the master branch merges for versioning PR. I will be working more today and we can get things merged and caught up to one another.

With the PR I will consolidate the const file and make sure the merge goes smooth too. Looking forward to the updates.

@olixr
Copy link
Member

olixr commented Nov 11, 2020

I created the PR #16 and will be working to merge this today. This way I can continue some work and keep our changes aligned without too many future conflicts.

I will report back some tests on my units with various sensors and features enabled to see that everything is working fine.

@olixr
Copy link
Member

olixr commented Nov 11, 2020

PR merged into feature branch to continue local testing.

@olixr
Copy link
Member

olixr commented Nov 12, 2020

Fixes being published onfeature as I get through tests. Feel free to review them with time. Going to need some help finalizing before merging to master.

Had to fix some items like imports and cleaned up workers more from the transition. Still working through them. So far most items have been functioning as expected.

I should note I needed sudo apt-get install libgpiod2 for the DHT changes. This will be added to the installer.

I am still working to resolve more issues with DHT. I have a DHT22 on pin 24 that was initially having issues during the setup. Getting Unable to set line 24 to input error. Not sure the cause and a simple pulling of the sensor and reattaching resolved the bad setup. I am still getting this error and sometimes the sensor boots up returns a good reading and then the following reads are the line input error and it never fixes itself.

The other thing I noticed is the shutdown process is hanging a little longer than usual. Seems like the linux sensor_worker was hanging longer. I ran the system a few times with the DHT properly engaged returning readings and then the system kept hanging waiting for the sensor worker to close.

In the humidity_sensor I see the following:

for i in range(15):

What is the purpose of the loop here? If its just to attempt a reading until a good result returns this is possibly the reason for the hanging.

I am going to dig into the shutdown process and work to clean that up more so things can hopefully terminate sooner.

Testing Progress:

  • Test on Raspberry Pi 4:
    • DHT sensors (Needs further Review)
      • DHT22 (Hangs on Exit & Buggy)
      • DHT11
    • Float sensor
    • BME680 I2C sensor
    • Relays
    • Controls (switch and button)
    • Display (PCF Backpack)
    • Triggers
    • Actions
    • Automations

@olixr
Copy link
Member

olixr commented Nov 12, 2020

A quick update on the DHT. The issue is repeatable a few times now.

Booting the system initalized the sensor and it returns one reading. The next reading is always this Unable to set line 24 to input error. Then nothing. The sensor worker also will never terminate and is being blocked somwhere.

Here is my output:

 __  __           _ _____ _
|  \/  |         | |  __ (_)
| \  / |_   _  __| | |__) |
| |\/| | | | |/ _` |  ___/ |
| |  | | |_| | (_| | |   | |
|_|  |_|\__,_|\__,_|_|   |_|
_________________________________________________

Eric Davisson @theDavisson
https://mudpi.app
Version:  0.9.1

Initializing Logger...			 Complete
Initializing Garden Control...		 Complete
Preparing Threads for Workers...	 Complete
Pi Camera...				 Disabled
LCD Display Worker...			 Initializing
LCD Displays...				 Initializing
Sensors...				 Initializing
I2C Comms...				 Initializing
Relay Worker pump_1...			 Initializing
Relay Worker pump_2...			 Initializing
Relay Worker valve_1...			 Initializing
Relay Worker valve_2...			 Initializing
18 Actions...				 Initializing
1 Sequences...				 Initializing
Triggers...				 Initializing
MudPi Node Workers...			 Disabled
MudPi Garden Controls...		 Initialized
Engaging MudPi Workers...
LCD Display Worker ...			 Online
Pi Sensor Worker [1 Sensors]...		 Online
Pi I2C Sensor Worker [1 Sensors]...	 Online
Relay Worker pump_1...			 Online
Relay Worker pump_2...			 Online
Relay Worker valve_1...			 Online
Relay Worker valve_2...			 Online
Sequence Worker [Example Sequence]...	 Online
Trigger Worker [5 Triggers]...		 Online
MudPi Garden Control...			 Online
_________________________________________________
De-initializing self.pulse_in
{'dht': {'temperature': 73.58, 'humidity': 41.5}}
{'barometer': {'temperature': 64.52, 'humidity': 40.6, 'pressure': 992.27, 'gas': 278008, 'altitude': 176.147}}
{'barometer': {'temperature': 64.5, 'humidity': 40.6, 'pressure': 992.28, 'gas': 41000, 'altitude': 176.084}}
Unable to set line 24 to input
{'barometer': {'temperature': 64.49, 'humidity': 40.6, 'pressure': 992.27, 'gas': 47880, 'altitude': 176.179}}

Using DHT22 on pin 24 marked as D24 in the config.

@yeyeto2788
Copy link
Contributor

yeyeto2788 commented Nov 12, 2020

@olixr awesome that most of the functionalities are working, the reading 15 times I don't actually see it as an issue but I do see it excessive.

I would recommend to change the whole code here since we should initialize the pin only once and then perform the readings (for loop), I would change also to make the reading of the sensor using the measure method. Take into account that the sampling rate of the DHT22/DHT11 is about 2 seconds (https://www.adafruit.com/product/385)

I'm not sure why there was a need to install libgpiod2 since on installation steps for the raspberry pi that package is not needed https://github.com/adafruit/Raspberry-Pi-Installer-Scripts/blob/master/libgpiod.sh

UPDATE:

  • From what I have found seems like this is a known DHT issue
  • I have filed a PR with an execution order changed to your branch.

@SecT0uch
Copy link
Contributor Author

Hey guys,

Glad to hear most of the changes work as expected.

I am still working to resolve more issues with DHT. I have a DHT22 on pin 24 that was initially having issues during the setup. Getting Unable to set line 24 to input error. Not sure the cause and a simple pulling of the sensor and reattaching resolved the bad setup. I am still getting this error and sometimes the sensor boots up returns a good reading and then the following reads are the line input error and it never fixes itself.

I faced this error too: It seems that we always need to (release the pins](https://github.com/mudpi/mudpi-core/blob/2fcbaf88e4a2ab93de410d833e6bcdcb2446a878/sensors/linux/humidity_sensor.py#L57], otherwise libgpiod never releases it.
Rebooting also fixes the issue.

Try changing:

for i in range(15):
    dhtDevice = self.sensor(self.pin_obj)

    try:
        temperature_c = dhtDevice.temperature
        humidity = dhtDevice.humidity
        if humidity is not None and temperature_c is not None:
            dhtDevice.exit()
            break

to something like:

for i in range(15):
    dhtDevice = self.sensor(self.pin_obj)

    try:
        temperature_c = dhtDevice.temperature
        humidity = dhtDevice.humidity
        if humidity is not None and temperature_c is not None:
            dhtDevice.exit()
            break
    except Exception:
        dhtDevice.exit()

dhtDevice.exit()
for i in range(15):

What is the purpose of the loop here? If its just to attempt a reading until a good result returns this is possibly the reason for the hanging.

In the previous version, you were using read_retry() from RPi.GPIO, which does exaclty what I reproduced here (if I'm not mistaken). There is no equivalent with the new lib, I did that to be sure to reproduce the same behavior.
Feel free to remove it if you don't think it's necessary.

@SecT0uch
Copy link
Contributor Author

From what I have found seems like this is a known DHT issue

@yeyeto2788 Yeah, I actually wrote a comment on the related PR suggesting a few changes.

@olixr
Copy link
Member

olixr commented Nov 13, 2020

I have been doing some debugging to try and figure a better solution. Unfortunately the library doesn't throw many errors and blocks often which makes this even harder.

The main issue I have is with the pulseio. In some cases it will corrupt and because of no exceptions its hard to recover from. Running a manual sudo pkill libgpiod_pulsei will release the problem and allow the program to run. However you can see how this is not ideal for letting the system recover on its own.

The other main problem is again the sensor seems to only read one time then error out. Even when I kill the process and only allows 1 reading before breaking. With their previous library there was not as many issues so I hope to get this integration working with DHT smoothly and figure out how to better get state of sensor or recover.

Also not a big deal but kinda annoying seeing De-initializing self.pulse_in every time getting printed by the library. I wish they would make that optional.

My goal right now is trying to initialize the sensor in MudPi and if it fails store the failed attempt and try to reinit on the next read. I was avoiding trying to init the sensor every read as this seemed a bit much. I need to figure out a way to capture the Unable to set line ## to input and use that for recovering. However it seems this is not an exception to catch and also blocks in some cases.

I have tried a few iterations of design and most of them are leading to the same mixed results. I have yet to get a smooth working DHT reading with the update. If the issue is in the library we might have to look into alternatives. Still trying to debug a solution.

@olixr
Copy link
Member

olixr commented Nov 13, 2020

I am thinking some sort of loop IS going to be the solution for now given how pulseio operates. When I run the example code it needs to loop in order to detect the sensor. The first read is usually sensor not found then it goes to normal operations. The other thing is after initiated once AND if I don't terminate the pulseio it seems to pick up the sensor right away on program restart. Now if I can get this behavior into MudPi.

This helps show that it is possible to work I am just working on how to get this integrated into the MudPi sensor cycle properly.

Here was the output of the example:

DHT sensor not found, check wiring
Temp: 79.5 F / 26.4 C    Humidity: 40.3%
Temp: 79.5 F / 26.4 C    Humidity: 40.3%
Temp: 79.5 F / 26.4 C    Humidity: 40.4%
Temp: 79.5 F / 26.4 C    Humidity: 40.5%
Temp: 79.5 F / 26.4 C    Humidity: 40.5%
Temp: 79.5 F / 26.4 C    Humidity: 40.5%
A full buffer was not returned. Try again.
Temp: 79.5 F / 26.4 C    Humidity: 40.5%
Temp: 79.5 F / 26.4 C    Humidity: 40.4%
Temp: 79.5 F / 26.4 C    Humidity: 40.3%
Temp: 79.7 F / 26.5 C    Humidity: 40.4%
Temp: 79.7 F / 26.5 C    Humidity: 40.4%
Temp: 79.7 F / 26.5 C    Humidity: 40.3%
Temp: 79.7 F / 26.5 C    Humidity: 39.4%
Temp: 79.7 F / 26.5 C    Humidity: 40.3%
Temp: 79.7 F / 26.5 C    Humidity: 40.3%
Temp: 79.7 F / 26.5 C    Humidity: 40.3%
Temp: 79.7 F / 26.5 C    Humidity: 40.3%
Temp: 79.7 F / 26.5 C    Humidity: 40.2%

@olixr
Copy link
Member

olixr commented Nov 13, 2020

Try the latest changes in feature and we can see where we are at. I'd say I am still getting mixed results but closer now as I have been able to get several successful readings. Even after a restart it worked again so the bugs are being cut down.

I am happy with the latest changes after running MudPi several times and trying to test various situations. For me I have a pretty solid working version of the humidity sensor for now. I had to play around with the exit() command a bit. Having the exit after each read wasn't working. It appears with it being used just as the Error catch its working. Also with the properties of temp and humidity it calls measure() behind the scenes so we don't need to make a direct call to this method.

Ill have you two test a DHT sensor and let me know how it runs now with the current feature branch.

@olixr
Copy link
Member

olixr commented Nov 13, 2020

I am doing remaining tests on the Pi. If one of you have other boards let me know when you have time to make some tests before the merge. I'll take care of the Pi side.

Testing Left Before Merge:

  • Test on Raspberry Pi:
    • DHT sensors
    • Float sensor
    • T9602 I2C sensor
    • BME680 I2C sensor
    • Relays
    • Displays
    • Controls (switch and button)
    • Triggers
    • Actions
    • Automation Sequences
    • Camera
  • Test on an armbian system:
    • DHT sensors
    • Float sensor
    • T9602 I2C sensor
    • BME680 I2C sensor
    • Relays (I did check, but we may want to double check that)
    • Controls (switch and button)
    • LCD
    • Triggers
    • Actions
    • Automation Sequences

@yeyeto2788
Copy link
Contributor

yeyeto2788 commented Nov 13, 2020

Hey @olixr,

I tried to test on Opi Lite (Armbian) the relay, sequence, action, and button but I got this error:

 __  __           _ _____ _
|  \/  |         | |  __ (_)
| \  / |_   _  __| | |__) |
| |\/| | | | |/ _` |  ___/ |
| |  | | |_| | (_| | |   | |
|_|  |_|\__,_|\__,_|_|   |_|
_________________________________________________

Eric Davisson @theDavisson
https://mudpi.app
Version:  0.8

Initializing Logger...                   Complete
Initializing Garden Control...           Complete
Preparing Threads for Workers...         Complete
Pi Camera...                             Disabled
MudPi Shutting Down...
MudPi Shutting Down...                   Complete
Mudpi is Now...                          Offline
Traceback (most recent call last):
  File "mudpi.py", line 198, in <module>
    pw = LinuxControlWorker(
  File "/home/yeyeto2788/workspace/mudpi-core/workers/linux/control_worker.py", line 19, in __init__
    self.init()
  File "/home/yeyeto2788/workspace/mudpi-core/workers/linux/control_worker.py", line 32, in init
    imported_control = self.dynamic_import(control_type)
  File "/home/yeyeto2788/workspace/mudpi-core/workers/linux/worker.py", line 68, in dynamic_import
    module = getattr(module, component)
AttributeError: module 'controls' has no attribute 'linux'

The configuration I used is this one:

{
  "name": "MudPi",
  "version": 0.8,
  "debug": false,
  "redis": {
    "host": "127.0.0.1",
    "port": 6666
  },
  "relays": [
    {
      "pin": "PA10",
      "normally_open": true,
      "group": "",
      "name": "Relay Name",
      "key": "freaking key needed",
      "topic": "garden/pi/relays/",
      "tag": "relay_1"
    }
  ],
  "actions": [
    {
      "type": "event",
      "name": "Toggle Pump ON",
      "key": "toggle_pump_on",
      "action": { "event": "Toggle" },
      "topic": "garden/pi/relays/1"
    },
    {
      "type": "event",
      "name": "Toggle Pump OFF",
      "key": "toggle_pump_off",
      "action": { "event": "Toggle" },
      "topic": "garden/pi/relays/0"
    }
  ],
  "sequences": [
    {
      "name": "Example Watering Sequence",
      "key": "example_sequence",
      "sequence": [
        {
          "actions": ["toggle_pump_on"],
          "duration": 2
        },
        {
          "actions": ["toggle_pump_off"],
          "duration": 2
        }
      ]
    }
  ],
  "triggers": [
    {
      "type": "time",
      "key": "timed_trigger",
      "name": "timed triggered sequence",
      "sequences": ["example_sequence"],
      "schedule": "* * * * *"
    }
  ],
  "workers": [
    {
      "type": "control",
      "controls": [
        {
          "type": "Button",
          "pin": "PG7",
          "key": "button_1"
        }
      ]
    }
  ],
  "nodes": []
}

It is weird that we do that kind of import. Is it possible to refactor that part?

Maybe it's just me configuring wrongly the core. 😛

@SecT0uch
Copy link
Contributor Author

Hey guys,

I just had a mountain of work coming in, and will be quite busy this week-end.
I will try to test all that when I find some time. (On mi Pi Zero W first, and on my SoPine + baseboard later on).

@olixr
Copy link
Member

olixr commented Nov 13, 2020

@yeyeto2788 Try the latest push I just did to feature. Its seems like we just had more missing imports.

So when we took out most of that sys.append it means we needed more explicit imports for the files. So in the workers I have been making sure to import the sensors or controls.

I updated the control worker with

from controls.linux.button_control import (ButtonControl)
from controls.linux.switch_control import (SwitchControl)

This is what your error was talking about and should resolve the problem.

@SecT0uch no worries. When you have time post what you can test. If we can test one 1 or 2 boards other than the Pi that would be great. I don't expect us to test every board now possible.

@SecT0uch
Copy link
Contributor Author

Also not a big deal but kinda annoying seeing De-initializing self.pulse_in every time getting printed by the library. I wish they would make that optional.

Just added a PR upstream to fix that. :)

@yeyeto2788
Copy link
Contributor

@olixr I'll pull the changes tomorrow, if by any chance you pull also @SecT0uch changes I can test on my board the code.

@yeyeto2788
Copy link
Contributor

Hello guys!

I finally got it to work but I actually don't see the relay going on and off. This is the output of the code running successfully:

 __  __           _ _____ _
|  \/  |         | |  __ (_)
| \  / |_   _  __| | |__) |
| |\/| | | | |/ _` |  ___/ |
| |  | | |_| | (_| | |   | |
|_|  |_|\__,_|\__,_|_|   |_|
_________________________________________________

Eric Davisson @theDavisson
https://mudpi.app
Version:  0.8

Initializing Logger...                   Complete
Initializing Garden Control...           Complete
Preparing Threads for Workers...         Complete
Pi Camera...                             Disabled
Button Control PG7...                    Ready
Controls...                              Initializing
Relay Worker freaking_key_needed...                      Initializing
2 Actions...                             Initializing
1 Sequences...                           Initializing
Triggers...                              Initializing
MudPi Garden Controls...                 Initialized
Engaging MudPi Workers...
Pi Control Worker [1 Controls]...        Online
Relay Worker freaking_key_needed...                      Online
Sequence Worker [Example Watering Sequence]...   Online
Trigger Worker [1 Triggers]...           Online
MudPi Garden Control...                  Online
_________________________________________________
Sequence Example Watering Sequence Started
Sequence Example Watering Sequence Started
^CMudPi Shutting Down...
Relay Worker freaking_key_needed Shutting Down...        Complete
Pi Control Worker Shutting Down...       Complete
Sequence Worker Shutting Down...         Complete
Trigger Worker Shutting Down...          Complete
MudPi Shutting Down...                   Complete
Mudpi is Now...                          Offline

I filed a PR due to missing dependency and a typo on pin argument getter.

@olixr
Copy link
Member

olixr commented Mar 12, 2021

After a few months of work I am back with updates. I took off from where we started on this and decided to just do the full system overhaul I have been wanting.

In the process I have completely separated out the need for RPIGPIO. This now opens up MudPi to run all linux boards. I also made it so the GPIO isnt event a hard requirement. Instead areas of the system that are not supported will just disable allowing everything else to run. I am now able to run MudPi on my mac even for testing which is great. It just gets the board errors and then disables the GPIO components as needed.

Check out the big updates: 809f998

I will be working on docs and testing before the final merge. I think though this covers the original adding more board support though. I will be closing this after some more tests.

@olixr
Copy link
Member

olixr commented Mar 12, 2021

Board support added. Closing original issue.

@SecT0uch
Copy link
Contributor Author

SecT0uch commented Apr 19, 2021

Just a small update to say I've been using the feature branch of MudPi for a week without issues on a Sopine + baseboard running armbian.

Here is my working configuration:

{
  "mudpi": {
    "name": "MudPi Example",
    "debug": true,
    "unit_system": "metric",
    "events": {
      "redis": {
        "host": "127.0.0.1",
        "port": 6379
      }
    }
  },
  "sensor": [
    {
      "interface": "t9602",
      "address": 40,
      "key": "temp_hum_sensor",
      "name": "Climate Sensor T9602"
    }
  ],
  "trigger": [
    {
      "key": "humidity_low",
      "interface": "state",
      "source": "temp_hum_sensor",
      "nested_source": "humidity",
      "name": "Humidity is low",
      "frequency": "once",
      "actions": [
        ".humidifier.turn_on"
      ],
      "thresholds": [
        {
          "comparison": "lt",
          "value": 43
        }
      ]
    },
    {
      "key": "humidity_high",
      "interface": "state",
      "source": "temp_hum_sensor",
      "nested_source": "humidity",
      "name": "Humidity is high",
      "frequency": "once",
      "actions": [
        ".humidifier.turn_off"
      ],
      "thresholds": [
        {
          "comparison": "gt",
          "value": 57
        }
      ]
    },
    {
      "key": "every_min_day",
      "interface": "cron",
      "schedule": "* 6-23 * * *"
    },
    {
      "key": "every_min_night",
      "interface": "cron",
      "schedule": "* 0-5 * * *"
    },
    {
      "key": "check_temp_min_day",
      "interface": "state",
      "source": "temp_hum_sensor",
      "nested_source": "temperature",
      "name": "Check min allowed temp during day",
      "frequency": "once",
      "thresholds": [
        {
          "comparison": "lt",
          "value": 25
        }
      ]
    },
    {
      "key": "check_temp_max_day",
      "interface": "state",
      "source": "temp_hum_sensor",
      "nested_source": "temperature",
      "name": "Check max allowed temp during day",
      "frequency": "once",
      "thresholds": [
        {
          "comparison": "gt",
          "value": 30
        }
      ]
    },
    {
      "key": "check_temp_min_night",
      "interface": "state",
      "source": "temp_hum_sensor",
      "nested_source": "temperature",
      "name": "Check min allowed temp during night",
      "frequency": "once",
      "thresholds": [
        {
          "comparison": "lt",
          "value": 22
        }
      ]
    },
    {
      "key": "check_temp_max_night",
      "interface": "state",
      "source": "temp_hum_sensor",
      "nested_source": "temperature",
      "name": "Check max allowed temp during night",
      "frequency": "once",
      "thresholds": [
        {
          "comparison": "gt",
          "value": 27
        }
      ]
    },
    {
      "key": "day_temp_low",
      "interface": "group",
      "name": "Day temperature is low",
      "actions": [
        ".heating.turn_on"
      ],
      "triggers": [
        "every_min_day",
        "check_temp_min_day"
      ]
    },
    {
      "key": "day_temp_high",
      "interface": "group",
      "name": "Day temperature is high",
      "actions": [
        ".heating.turn_off"
      ],
      "triggers": [
        "every_min_day",
        "check_temp_max_day"
      ]
    },
    {
      "key": "night_temp_low",
      "interface": "group",
      "name": "Night temperature is low",
      "actions": [
        ".heating.turn_on"
      ],
      "triggers": [
        "every_min_night",
        "check_temp_min_night"
      ]
    },
    {
      "key": "night_temp_high",
      "interface": "group",
      "name": "Night temperature is high",
      "actions": [
        ".heating.turn_off"
      ],
      "triggers": [
        "every_min_night",
        "check_temp_max_night"
      ]
    }
  ],
  "toggle": [
    {
      "key": "light",
      "interface": "gpio",
      "pin": "D26",
      "invert_state": true,
      "name": "Light - Plug 1"
    },
    {
      "key": "humidifier",
      "interface": "gpio",
      "pin": "D19",
      "invert_state": true,
      "name": "Humidifier - Plug 2"
    },
    {
      "key": "heating",
      "interface": "gpio",
      "pin": "D13",
      "invert_state": true,
      "name": "Heating - Plug 3"
    }
  ]
}

@olixr I'm not 100% sure it's the intended way to control a parameter with different settings during the day and at night.
In this example, I set the temperature between 25 and 30°C during the day and between 22 and 27°C during the night.

Is there any other way to achieve that ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants