openenergymonitor / emonpi

Raspberry Pi Based Energy Monitor. Hardware, Firmware & related software for the PI.
https://guide.openenergymonitor.org/setup
270 stars 113 forks source link

Port LCD to python3 #120

Closed bwduncan closed 4 years ago

bwduncan commented 4 years ago

This isn't ready for release yet, I should have used a different branch name :shrug: but this PR could be a handy way to consolidate comments.

borpin commented 4 years ago

Re https://community.openenergymonitor.org/t/python3-for-emonhub/12670/79

Install process needs to be updated to accommodate this. https://github.com/openenergymonitor/EmonScripts/issues/93

@TrystanLea,

  1. Update the emonScripts repo to point the install to the emonpi/lcd folder
  2. Create an install file in emonpi/lcd 'as is'.
  3. Publish both repos as an update.

Once done, as part of the PR for Python3

  1. update install file for python3 and modify how user, log etc are created.
  2. update the service file for Python3

Publishing the Python3 updated emonpi repo will then pull in the right install script and service file for Python3.

alexandrecuer commented 4 years ago

OK what about this :

0) addition of a python_version var in the emonsd.config.ini file from the EmonScripts

python_version=3

1) emonpilcd.sh file from the EmonScripts could become

#!/bin/bash
source load_config.sh

echo "-------------------------------------------------------------"
echo "emonpiLCD install"
echo "-------------------------------------------------------------"

if [ ! -d /var/log/emonpilcd ]; then
    # emonPiLCD Logger
    sudo mkdir /var/log/emonpilcd
    sudo chown $user /var/log/emonpilcd
    # Permissions?
    touch /var/log/emonpilcd/emonpilcd.log
fi

cd $openenergymonitor_dir

if [ ! -d $openenergymonitor_dir/emonpi ]; then
    git clone ${git_repo[emonpi]}
else
    echo "- emonpi repository already installed"
    git pull
fi

if [ -f $openenergymonitor_dir/emonpi/lcd/install.sh ]; then
    $openenergymonitor_dir/emonpi/lcd/install.sh $python_version
else
    echo "ERROR: $openenergymonitor_dir/emonpi/lcd/install.sh script does not exist"
fi

2) creation of an install.sh file in the lcd folder of the emonpi repo :

#!/bin/bash
# -------------------------------------------------------------
# emonpilcd install script
# -------------------------------------------------------------
# Assumes emonpi repository installed via git:
# git clone https://github.com/openenergymonitor/emonpi.git

DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
usrdir=${DIR/\/emonpi/}

python_version=$1

if [ "$python_version" = 3 ]; then
    sudo apt-get install python3-smbus i2c-tools python3-rpi.gpio python3-pip redis-server  python3-gpiozero -y
    pip3 install redis paho-mqtt xmltodict requests
fi

if [ "$python_version" = 2 ]; then
    sudo apt-get install -y python-smbus i2c-tools python-rpi.gpio python-gpiozero
    pip install xmltodict
fi

# Uncomment dtparam=i2c_arm=on
sudo sed -i "s/^#dtparam=i2c_arm=on/dtparam=i2c_arm=on/" /boot/config.txt
# Append line i2c-dev to /etc/modules
sudo sed -i -n '/i2c-dev/!p;$a i2c-dev' /etc/modules

# ---------------------------------------------------------
# Install service
# ---------------------------------------------------------
service=emonPiLCD

if [ -f /lib/systemd/system/$service.service ]; then
    echo "- reinstalling $service.service"
    sudo systemctl stop $service.service
    sudo systemctl disable $service.service
    sudo rm /lib/systemd/system/$service.service
else
    echo "- installing $service.service"
fi

# ---------------------------------------------------------
# Install service
# ---------------------------------------------------------
echo "- installing emonPiLCD.service"
sudo ln -sf $usrdir/emonpi/lcd/$service.service /lib/systemd/system
sudo systemctl enable $service.service
sudo systemctl restart $service.service

state=$(systemctl show $service | grep ActiveState)
echo "- Service $state"
# ---------------------------------------------------------

@TrystanLea @borpin : would you be happy with something like that ?

borpin commented 4 years ago
  1. addition of a python_version var in the emonsd.config.ini file from the EmonScripts

No it will just confuse things and the emonpi install is dependednt on the right emonscripts repo version.

If we move the install to the lcd folder and make the service file, the install & update scripts and the lcd python script all match in terms of python version it will just work as it has for emonhub (with a few glitches).

The emonscript then just calls the install (or update) script in the lcd folder and the right script get executed.

What we do need to handle is the update. In the existing python2.7 version, as well as moving the install script we create an update script that, for the 2.7 version, is empty. Also need to get the update process to call it. All that is part of the emonscripts update i.e. nothing actually changes, just the way things are called change.

When the python3 version of the repo is created, the update script is created. You could also add a version.txt to the repo to be checked.

It also has the advantage that, it you change branches of the emonpi repo, the install/update process will automatically pick up the right version of the lcd script.

borpin commented 4 years ago

It should not need to check for the emonpi folder. If it isn't there, something is far wrong!

Create log folder - do that in the main install script.

All it needs is to check for the install script and then run it.

It is just that both repos need to be updated at the same time (well emonpi first then emonscripts).

Cheers

bwduncan commented 4 years ago

Create log folder - do that in the main install script.

This should be done by the service as an ExecStartPre=. /var/log might be a tmpfs, so the service should depend on var-log.mount as well.

borpin commented 4 years ago

Just thinking that the way to know what version of Python to install would be by interrogating the hash bang of the script.

bwduncan commented 4 years ago

That's a possibility. We should be moving everything to python 3 though. Python 2 is out of support as of (last!) January.

Nobody keen to just package this as a deb which specifies its own dependencies and let apt handle it?

borpin commented 4 years ago

We should be moving everything to python 3 though.

Totally agree. For any release, if the install/update script matches the python script, then the right packages should be installed so a check isn't actually required.

Oh and always add a sudo apt update before any packages are installed!

alexandrecuer commented 4 years ago

Just to share something a little bit updated :

python version catched by interrogating the shebang of emonPiLCD.py + addition of a sudo apt update....So only two files are concerned : emonpilcd.sh from EmonScripts and the new install.sh in the lcd folder of the emonpi repo....

1) emonpilcd.sh file from the EmonScripts could become :

#!/bin/bash
source load_config.sh

echo "-------------------------------------------------------------"
echo "emonpiLCD install"
echo "-------------------------------------------------------------"

if [ ! -d /var/log/emonpilcd ]; then
    # emonPiLCD Logger
    sudo mkdir /var/log/emonpilcd
    sudo chown $user /var/log/emonpilcd
    # Permissions?
    touch /var/log/emonpilcd/emonpilcd.log
fi

cd $openenergymonitor_dir

if [ ! -d $openenergymonitor_dir/emonpi ]; then
    git clone ${git_repo[emonpi]}
else
    echo "- emonpi repository already installed"
    git pull
fi

if [ -f $openenergymonitor_dir/emonpi/install.sh ]; then
    $openenergymonitor_dir/emonpi/install.sh
else
    echo "ERROR: $openenergymonitor_dir/emonpi/install.sh script does not exist"
fi

It should not need to check for the emonpi folder. If it isn't there, something is far wrong!

the test on the emonpi folder in the $openenergymonitor_dir folder is just there to initialize the process on a 'blank' OS.....if for some reason, the scripted install was stopped and relaunched, we just make a git pull.....

2) creation of an install.sh file in the lcd folder of the emonpi repo :

#!/bin/bash
# -------------------------------------------------------------
# emonpilcd install script
# -------------------------------------------------------------
# Assumes emonpi repository installed via git:
# git clone https://github.com/openenergymonitor/emonpi.git

DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
usrdir=${DIR/\/emonpi/}

# interrogates the shebang of emonPiLCD.py to catch the python version
shebang="$(head -1 emonPiLCD.py)"
splitted=($(echo $shebang | tr "/" "\n"))
python="${splitted[-1]}"

sudo apt update

if [ "$python" = "python3" ]; then
    sudo apt-get install python3-smbus i2c-tools python3-rpi.gpio python3-pip redis-server  python3-gpiozero -y
    pip3 install redis paho-mqtt xmltodict requests
else
    sudo apt-get install -y python-smbus i2c-tools python-rpi.gpio python-gpiozero
    pip install xmltodict
fi

# Uncomment dtparam=i2c_arm=on
sudo sed -i "s/^#dtparam=i2c_arm=on/dtparam=i2c_arm=on/" /boot/config.txt
# Append line i2c-dev to /etc/modules
sudo sed -i -n '/i2c-dev/!p;$a i2c-dev' /etc/modules

# ---------------------------------------------------------
# Install service
# ---------------------------------------------------------
service=emonPiLCD

if [ -f /lib/systemd/system/$service.service ]; then
    echo "- reinstalling $service.service"
    sudo systemctl stop $service.service
    sudo systemctl disable $service.service
    sudo rm /lib/systemd/system/$service.service
else
    echo "- installing $service.service"
fi

# ---------------------------------------------------------
# Install service
# ---------------------------------------------------------
echo "- installing emonPiLCD.service"
sudo ln -sf $usrdir/emonpi/lcd/$service.service /lib/systemd/system
sudo systemctl enable $service.service
sudo systemctl restart $service.service

state=$(systemctl show $service | grep ActiveState)
echo "- Service $state"
# ---------------------------------------------------------

Nobody keen to just package this as a deb which specifies its own dependencies and let apt handle it?

could be an interesting approach :+1:

The emonscript then just calls the install (or update) script in the lcd folder and the right script get executed.

What we do need to handle is the update. In the existing python2.7 version, as well as moving the install script we create an update script that, for the 2.7 version, is empty. Also need to get the update process to call it. All that is part of the emonscripts update i.e. nothing actually changes, just the way things are called change.

When the python3 version of the repo is created, the update script is created. You could also add a version.txt to the repo to be checked.

Sorry, but this is not clear for me : a new python3 branch in the repo emonpi (if so we do not need to track the python version) ? an update script in the update directory of the repo EmonScripts ?

TO DO : define where the log folder is created

alexandrecuer commented 4 years ago

Updating my proposition so that the service file can create the log... everything in the emonhub way....

1) emonPiLCD.service :

[Unit]
Description=emonPiLCD service description
After=mosquitto.service

[Service]
User=pi
PIDFile=/var/run/emonpi.pid
ExecStart=/opt/openenergymonitor/emonpi/lcd/emonPiLCD.py --logfile=/var/log/emonpilcd/emonpilcd.log
PermissionsStartOnly=true
ExecStartPre=/bin/mkdir -p /var/log/emonpilcd/
ExecStartPre=/bin/chgrp -R pi /var/log/emonpilcd/
ExecStartPre=/bin/chmod 775 /var/log/emonpilcd/
Type=simple
Restart=always

[Install]
WantedBy=multi-user.target

2) creation of an install.sh file in the lcd folder of the emonpi repo :

#!/bin/bash
# -------------------------------------------------------------
# emonpilcd install script
# -------------------------------------------------------------
# Assumes emonpi repository installed via git:
# git clone https://github.com/openenergymonitor/emonpi.git

usrdir="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"

# interrogates the shebang of emonPiLCD.py to catch the python version
shebang="$(head -1 emonPiLCD.py)"
splitted=($(echo $shebang | tr "/" "\n"))
python="${splitted[-1]}"

sudo apt update

if [ "$python" = "python3" ]; then
    sudo apt-get install python3-smbus i2c-tools python3-rpi.gpio python3-pip redis-server  python3-gpiozero -y
    pip3 install redis paho-mqtt xmltodict requests
else
    sudo apt-get install -y python-smbus i2c-tools python-rpi.gpio python-gpiozero
    pip install redis paho-mqtt xmltodict requests
fi

# Uncomment dtparam=i2c_arm=on
sudo sed -i "s/^#dtparam=i2c_arm=on/dtparam=i2c_arm=on/" /boot/config.txt
# Append line i2c-dev to /etc/modules
sudo sed -i -n '/i2c-dev/!p;$a i2c-dev' /etc/modules

# ---------------------------------------------------------
# Install service
# ---------------------------------------------------------
service=emonPiLCD

if [ -f /lib/systemd/system/$service.service ]; then
    echo "- reinstalling $service.service"
    sudo systemctl stop $service.service
    sudo systemctl disable $service.service
    sudo rm /lib/systemd/system/$service.service
else
    echo "- installing $service.service"
fi

# ---------------------------------------------------------
# Install service
# ---------------------------------------------------------
echo "- installing emonPiLCD.service"
sudo ln -sf $usrdir/$service.service /lib/systemd/system
sudo systemctl enable $service.service
sudo systemctl restart $service.service

state=$(systemctl show $service | grep ActiveState)
echo "- Service $state"
# ---------------------------------------------------------

3) emonpilcd.sh file from the EmonScripts

#!/bin/bash
source load_config.sh

echo "-------------------------------------------------------------"
echo "emonpiLCD install"
echo "-------------------------------------------------------------"
cd $openenergymonitor_dir

if [ ! -d $openenergymonitor_dir/emonpi ]; then
    git clone ${git_repo[emonpi]}
else
    echo "- emonpi repository already installed"
    git pull
fi

if [ -f $openenergymonitor_dir/emonpi/lcd/install.sh ]; then
    $openenergymonitor_dir/emonpi/lcd/install.sh
else
    echo "ERROR: $openenergymonitor_dir/emonpi/lcd/install.sh script does not exist"
fi
borpin commented 4 years ago

There is no need to check for the python version.

Create this method for current script, the install will use python. Update this repo. modify the emonscripts script and update emonscripts.

When the python3 version of LCD is released, it should include a python3 version of the install.

Note, the service file should be as per these service files https://github.com/emoncms/emoncms/blob/28fb790b1aba5445f0b6c984e555e5502e491af2/scripts/services/feedwriter/feedwriter.service#L46

The install of the service file also needs to account for non-raspberry installs.

Suggest you do this as a PR for here (service file and install script) and a PR in emonscripts.

The Python 3 PR then needs to include the changes needed to the install script.

alexandrecuer commented 4 years ago

The install of the service file also needs to account for non-raspberry installs.

The emonpi repo is only for emonpi so only for raspberry platform, no?

borpin commented 4 years ago

The emonpi repo is only for emonpi so only for raspberry platform, no?

Ah yes of course šŸ˜

TrystanLea commented 4 years ago

Thanks @bwduncan for all of your work on this. Just working through testing now that I've merged @alexandrecuer's pull request.

There is a conflict in the service file and I wondered what you both think should be present in the resolved version:

@bwduncan's version:

# Systemd unit file for emonPiLCD script

# INSTALL:
# sudo ln -s /usr/share/emonPiLCD/emonPiLCD.service /lib/systemd/system

# RUN AT STARTUP
# sudo systemctl daemon-reload
# sudo systemctl enable emonPiLCD.service

# START / STOP With:
# sudo systemctl start emonPiLCD
# sudo systemctl stop emonPiLCD

# VIEW STATUS
# systemctl status emonPiLCD
# VIEW LOG
# journalctl -f -u emonPiLCD

[Unit]
Description=emonPi LCD driver
Wants=mosquitto.service
After=mosquitto.service

# Needs to run as root so disable/enable ssh and shutdown emonpi
[Service]
Type=idle
ExecStart=/usr/bin/python3 /usr/share/emonPiLCD/emonPiLCD.py
User=root

# Restart script if stopped on a failure. Will not restart if not configured correctly
Restart=on-failure
# Wait 60s before restart
RestartSec=60

# Tag things in the log
# If you want to use the journal instead of the file above, uncomment SyslogIdentifier below
# View with: sudo journalctl -f -u emonPiLCD -o cat
SyslogIdentifier=emonPiLCD

[Install]
WantedBy=multi-user.target

@alexandrecuer's version:

# Systemd unit file for emonPiLCD script
# installation is done via the install.sh script in the same folder
# VIEW STATUS
# systemctl status emonPiLCD
# VIEW LOG
# journalctl -f -u emonPiLCD

[Unit]
Description=emonPiLCD service description
After=mosquitto.service

[Service]
User=pi
PIDFile=/var/run/emonpilcd.pid
Environment='LOG_PATH=/var/log/emonpilcd'
ExecStart=/opt/openenergymonitor/emonpi/lcd/emonPiLCD.py --logfile=${LOG_PATH}/emonpilcd.log
PermissionsStartOnly=true
ExecStartPre=/bin/mkdir -p ${LOG_PATH}/
ExecStartPre=/bin/chgrp -R pi ${LOG_PATH}/
ExecStartPre=/bin/chmod 775 ${LOG_PATH}/
Type=simple
Restart=always

[Install]
WantedBy=multi-user.target
TrystanLea commented 4 years ago

I've merged this pull request into a branch called python3_lcd including an initial resolution of the service file conflict:

https://github.com/openenergymonitor/emonpi/tree/python3_lcd

https://github.com/openenergymonitor/emonpi/blob/python3_lcd/lcd/emonPiLCD.service

bwduncan commented 4 years ago

Definitely Alex's version, but with the description from mine if you can be bothered. Thanks!

On Mon, 3 Aug 2020, 09:44 Trystan Lea, notifications@github.com wrote:

Thanks @bwduncan https://github.com/bwduncan for all of your work on this. Just working through testing now that I've merged @alexandrecuer https://github.com/alexandrecuer's pull request.

There is a conflict in the service file and I wondered what you both think should be present in the resolved version:

@bwduncan https://github.com/bwduncan's version:

Systemd unit file for emonPiLCD script

INSTALL:

sudo ln -s /usr/share/emonPiLCD/emonPiLCD.service /lib/systemd/system

RUN AT STARTUP

sudo systemctl daemon-reload

sudo systemctl enable emonPiLCD.service

START / STOP With:

sudo systemctl start emonPiLCD

sudo systemctl stop emonPiLCD

VIEW STATUS

systemctl status emonPiLCD

VIEW LOG

journalctl -f -u emonPiLCD

[Unit] Description=emonPi LCD driver Wants=mosquitto.service After=mosquitto.service

Needs to run as root so disable/enable ssh and shutdown emonpi

[Service] Type=idle ExecStart=/usr/bin/python3 /usr/share/emonPiLCD/emonPiLCD.py User=root

Restart script if stopped on a failure. Will not restart if not configured correctly

Restart=on-failure

Wait 60s before restart

RestartSec=60

Tag things in the log

If you want to use the journal instead of the file above, uncomment SyslogIdentifier below

View with: sudo journalctl -f -u emonPiLCD -o cat

SyslogIdentifier=emonPiLCD

[Install] WantedBy=multi-user.target

@alexandrecuer https://github.com/alexandrecuer's version:

Systemd unit file for emonPiLCD script

installation is done via the install.sh script in the same folder

VIEW STATUS

systemctl status emonPiLCD

VIEW LOG

journalctl -f -u emonPiLCD

[Unit] Description=emonPiLCD service description After=mosquitto.service

[Service] User=pi PIDFile=/var/run/emonpilcd.pid Environment='LOG_PATH=/var/log/emonpilcd' ExecStart=/opt/openenergymonitor/emonpi/lcd/emonPiLCD.py --logfile=${LOG_PATH}/emonpilcd.log PermissionsStartOnly=true ExecStartPre=/bin/mkdir -p ${LOG_PATH}/ ExecStartPre=/bin/chgrp -R pi ${LOG_PATH}/ ExecStartPre=/bin/chmod 775 ${LOG_PATH}/ Type=simple Restart=always

[Install] WantedBy=multi-user.target

ā€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openenergymonitor/emonpi/pull/120#issuecomment-667893473, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADSULHIS524ED65OR4WHCTR6Z2FXANCNFSM4N5VZKIA .

borpin commented 4 years ago

No need for a drop in as this service is only any use on a Pi!

TrystanLea commented 4 years ago

Thanks @bwduncan I've update the service description.

For some reason testing this here, I dont get anything printed to the LCD, when I revert back to running a new script I've added with python2 it seems to print sometimes and not others.

Never prints anything:

python3 emonPiLCD.py
python3 emonPiLCD_write.py test test

Works intermittently:

python emonPiLCD_write.py test test
TrystanLea commented 4 years ago

When I was testing earlier with python2 I didnt see any of these issues, so I dont think its the LCD hardware. I reckon there is something in the updated libraries, but then that shouldnt have affected running using python2.. strange..

bwduncan commented 4 years ago

I'm on a bus now heading off to my holidays so I'm afraid I won't be able to spend much time on this this week. With any luck I'll have no phone signal at all šŸ˜‰

However, I did make some changes to the LCD libraries, to remove unused code, remove a layer of indirection and add support for custom characters. It's possible that the API has changed in a way your test script wasn't expecting. Can you post your script?

Bruce

On Mon, 3 Aug 2020, 10:15 Trystan Lea, notifications@github.com wrote:

When I was testing earlier with python2 I didnt see any of these issues, so I dont think its the LCD hardware. I reckon there is something in the updated libraries, but then that shouldnt have affected running using python2.. strange..

ā€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openenergymonitor/emonpi/pull/120#issuecomment-667909091, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADSULASUBF4KAJU7N6SXTTR6Z53DANCNFSM4N5VZKIA .

TrystanLea commented 4 years ago

Thanks @bwduncan, ah dont worry, have a nice holiday!! Are you going to the mountains somewhere?

Here's the test script: https://github.com/openenergymonitor/emonpi/blob/master/lcd/emonPiLCD_write.py

the part that prints to the LCD is:

lcd = lcddriver.lcd(int(address, 16))
lcd.backlight = 1
lcd.lcd_display_string(line1,1)
lcd.lcd_display_string(line2,2)

but this issue affects the main emonpiLCD.py script as well.

Im just trying a fresh install here to see if the issue persists.

alexandrecuer commented 4 years ago

Cannot test right now as i am far from office...in holidays :-)....with no emonpi nearby...

previously I proceeded to some testings with both Bruce python3 emonpi version and the emonpilcd service file in the 'emonhub' mood (ie as recently merged by Trystan)....

indeed, there were some problems : the lcd could remain blank as described by Trystan....

if i remember well, everything was going fine (without too much effort....) till Bruce last commit on his fork (15 of june)

the last commit i could successfully test is this one I think : https://github.com/bwduncan/emonpi/tree/b7c856e40b3820a54466ccd69a5e71b72f4be021

When I get back, I'll run some tests with Bruce last commits but you might find the solution before ;-)

Best Alex

TrystanLea commented 4 years ago

Seems there was a very slight timing related issue, introducing a small delay here seems to have sorted it: https://github.com/openenergymonitor/emonpi/commit/0c85f82ac4d177a5ef73dcb9cb7e1d5270966cbb

Should I introduce a delay after all writes? It seems to be working as it, perhaps we introduce a delay to the other places only if an issue is presented?

bwduncan commented 4 years ago

That's really weird because there is already a delay in lcd_write_four_bits. But hey if it works... I tried to find a data sheet which gave the timings for these iĀ²c transactions but I got lost in a myriad of documents and gave up

On Mon, 3 Aug 2020, 15:09 Trystan Lea, notifications@github.com wrote:

Seems there was a very slight timing related issue, introducing a small delay here seems to have sorted it: 0c85f82 https://github.com/openenergymonitor/emonpi/commit/0c85f82ac4d177a5ef73dcb9cb7e1d5270966cbb

Should I introduce a delay after all writes? It seems to be working as it, perhaps we introduce a delay to the other places only if an issue is presented?

ā€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openenergymonitor/emonpi/pull/120#issuecomment-668043726, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADSULHCLCGZA2CU2JK7EETR63AJHANCNFSM4N5VZKIA .

TrystanLea commented 4 years ago

Thanks @bwduncan always hard to track these kinds of issues down, I wonder if there may be some variation in LCD hardware batch that may have resulted in it showing up this time, who knows! something to keep an eye on I guess