Lucid migration

[5iver]

Round 3 of migration. This should wrap up the bulk of the changes, and we are ready for testing. ideAlarm and weatherStationUpdloader are now in my fork. The lucid-migration branch in this repo is behind, so please use my fork for testing. We will also need to discuss/resolve everything below.

Things removed during lucid migration:

1. randomization of cron expressions (prefer manual configuration)
2. did not include pronounce (could be added with a Community TTS script)
3. did not include hasReloadFinished, since the StartupDelay script takes care of this
4. removed core.utils.curDateText since core.date.py has similar function
5. In rules.py, the globals injected into the function are not necessary, since they are already in the default scope. Log is already being added as an attribute to the action functions. I also do not see a need for the cron expressions, but I could be convinced that these could go into configuration.py. The event object concerns me. I don't think it is a good idea to be modifying this, and most of what is being added is either already there and being overwritten. isActive is a duplicate of what is found in utils.py (which I think should be moved... see 3), and hasReloadFinished is unecessary due to the startup delay script.

Things I think should be removed:

1. core.utils.postUpdate and sendCommand should be removed, since they are converting to string, but that is not always correct and will cause issues. All these do is remove the need to type 'events.' before the commands, so I also do not think they are necessary.
2. [DONE] Nothing in core should be using customItemNames. Anything using it can be moved to implementation specific modules in community.
3. core.utils.getItemValue does not cover all possible cases for Item type. If we keep it, this should be cleaned up, but I really do not see much value in it.
4. [pe was cleaned up] core.utils.getLastUpdate should be removed. I'm not clear on the useage, but it appears to require PersistenceExtension to be supplied as a parameter?
5. TimeOfDay example rule should be removed or corrected (use String instead of Number). I've added another example that could replace it.
6. [DONE] isActive, isBright, isShady, isDark, isBlack shouild all be moved out of utils and into an implementation specific script or module.

Other considerations:

1. StartupTrigger will not work until the fix is merged (after this one).
2. PUSHOVER_DEF_DEV was imported in ideAlarm, but it wasn't used and there was no definition for it.
3. Any concern with moving community.clickatell.sendsms to clickatell.init.py?
4. I've migrated weatherStationUploader, but it will not work since it uses wunderground, who have changed their API.
5. What to do with versioning? There has been some discussion here about approaches for this. I've bumped some versions, but definitely something to verify in the review.
6. I tried something in IdeAlarm.getTriggers(), which wasn't really unecessary but will allow use of the decorators for rule definitions. Definitely needs testing.
7. postUpdateCheckFirst should use TypeParser.parseCommand and TypeParser.parseState.

Fixes #51, #52, #53.

[besynnerlig]

Thanks Scott.

I will dive into this as soon as you have finished and I will answer to each comment that you've made of course.

Encountered new problems using my Windows git client on a SSH mounted shared directory on my linux server. I think I'll have to give that up. Maybe it was doomed to malfunction from start. I will have to figure out a better setup and a better workflow and do all the git stuff from the command line in linux instead.

Please let me know when you think it's a good time for me to start testing.

[besynnerlig]

@openhab-5iver

A few questions...

Should I test this in OH2.4 stable?

I was thinking that the best test would be to use all my current scripts including ideAlarm. Do we really need a separate branch for ideAlarm or can we try to incorporate it into the Lucid branch? What would you prefer?

[besynnerlig]

Things I think should be removed:

1. I don't think configuration.postUpdate and sendCommand should be included, since they are converting to string, but that is not always needed and may cause issues

Unless I convert it to a string I'll get the following error:

TypeError: sendCommand(): 1st arg can't be coerced to String, org.eclipse.smarthome.core.items.Item

Using code

       from lucid.jsr223.scope import events
       events.sendCommand("Test_Number", 1)

Even though Test_Number is defined as a Number type Item.

[5iver]

Should I test this in OH2.4 stable?

Yes. Anything <= S1524. Builds between S1524 and S1566 had a bug that broke scripted automation and was resolved in S1566. But that build also included the ESH reintegration, where some packages were renamed and requires changes to the libraries. Once lucid is migrated, I will get some releases built so that people can easily pick a version of the libraries to suit their OH version.

Do we really need a separate branch for ideAlarm or can we try to incorporate it into the Lucid branch?

These can be merged. I'll do that today.

Unless I convert it to a string I'll get the following error:

To use sendCommand where the command is not a String, you need to use an Item. Similar for postUpdate.

events.sendCommand(ir.getItem("Test_Number"), 5)

https://www.openhab.org/docs/configuration/jsr223.html#events-operations

[5iver]

Encountered new problems using my Windows git client on a SSH mounted shared directory

Try a samba mount and VS Code. This works well for me for quick stuff (but I use Fedora for my OS... client and server), but I've also started using Eclipse with the Jython stuff. I has it's own quirks, but EGit has some benefits, particularly the interactive rebase.

[besynnerlig]

Try a samba mount and VS Code

Thanks I will try to set up a Samba mount tomorrow on my Ubuntu server.

[besynnerlig]

Things I think should be removed:

1. I don't think configuration.postUpdate and sendCommand should be included, since they are converting to string, but that is not always needed and may cause issues

Thanks for your explanation. I agree. But for backward compatibility and to prevent creating a breaking change, what if we just modify them like this:

def sendCommand(item, newValue):
    '''
    Sends a command to an item regerdless of it's current state
    The item can be passed as an OH item type or by using the item's name (string)
    '''
    events.sendCommand((itemRegistry.getItem(item) if isinstance(item, basestring) else item), newValue)

def postUpdate(item, newValue):
    '''
    Posts an update to an item regerdless of it's current state
    The item can be passed as an OH item type or by using the item's name (string)
    '''
    events.postUpdate((itemRegistry.getItem(item) if isinstance(item, basestring) else item), newValue)

About the postUpdateCheckFirst functions, what about if we change to something like this:

        if sendACommand:
            log.debug('New sendCommand value for '+itemName+' is '+str(newValue))
            events.sendCommand(item, newValue)
        else:
            log.debug('New postUpdate value for '+itemName+' is '+str(newValue))
            events.postUpdate(item, newValue)

I'll submit additional reviews (I hope that's possible, well, I'll find out) regarding the other comments.

EDIT: Code updated

[5iver]

But for backward compatibility and to prevent creating a breaking change, what if we just modify them like this:

I think that will resolve the issue, but I'm not clear on why you couldn't just send what is received.

My issue though, is that I question the need for this in the helper libraries at all, especially in core. The only convenience provided is to not have to type events., and I don't see that as such a terrible burden (:slightly_smiling_face:)! In the future, it may be possible to add the ScriptBusEvent methods as functions into the default scope, as is done for the DSL.

It's really not a big concern for me though, so I left them in!

[besynnerlig]

Have we now reached the point where it's time for me to start testing?

[5iver]

Yes... ready for initial smoke tests, but expect errors! I've tested some of the scripts for proper formatting, but nothing functional. It will be a lot easier for people who were already using the libraries to do the testing. It may be easier to correct errors after it is merged, but it would be nice to get as much as we can fixed now.

[besynnerlig]

Yes... ready for initial smoke tests, but expect errors! I've tested some of the scripts for proper formatting, but nothing functional. It will be a lot easier for people who were already using the libraries to do the testing. It may be easier to correct errors after it is merged, but it would be nice to get as much as we can fixed now.

Thanks a bunch!

The github pull requests extension in VSC is new to me and there is a learning threshold. I'll try to get used working with it and I'll see if I can start using code commenting as well.

One thing that happens if I checkout the pr/openhab-5iver/91* PR locally is that some recent changes in master are not there. (Missing .gitignore file and more...) I'll have to study how to handle that I guess or maybe the pull request can be rebased with master?

[5iver]

I'll try to get it synced up, but you could just download https://github.com/openhab-5iver/openhab2-jython/tree/lucid-migration and copy the files you need.

[besynnerlig]

I'm still a NOOB when it comes to github. I managed to get it synced locally but struggeled with merging conflicts due to differences regarding the "But-How-Do-I.md" whether it was renamed or deleted and git complained that I had to reolve that and commit before merge is possible. Anyway I got passed it but I probably messed up my local repo a bit. I might want to re-clone the original repo when/if you make the PR up to date with master.

I added a note in the getting started document about the need of creating a config file.

- You'll need a main configuration file for openhab2-jython. In directory `Core\automation\lib\python\`, rename the file `configuration.py.example` to `configuration.py` Then edit the configuration file to suit your needs.

I'm still trying to figure out a good workflow so that we don't have to make every single change 3 times. (I change, I write a comment here, you edit the original files). Maybe I need a web repo fork as an intermediate so that I can push my change proposals. Yeah, I think I will try to set that up tomorrow.

EDIT: I've now made forked openhab-5iver/openhab2-jython to a personal web repo and I sucessfylly merged the master branch into the lucid-migration branch. That worked well. After that I cloned my repo to my local server, checked out the lucid-migration branch (which is now up to date) and I start to push my changes to it. I guess that when I'm finished I can make a PR to your repo openhab-5iver/openhab2-jython Do you think that will be a good way forward?

I have one question though.

Previously I could change the logging level at rule level which I found very convenient.

@rule
class doorBell(object):
    """Someone rings the door bell"""
    def getEventTriggers(self):
        return [
            ItemStateUpdateTrigger('Door_Signal', str(ON)),
        ]
    def execute(self, modules, inputs):
        self.log.setLevel(DEBUG)

DEBUG is no longer in the global scope and even if I try to set the logging level for a rule it doesn't seem to make any difference.

I tried this but it didn't work out:

@rule("Hello World cron rule (decorator)", description="This is an example rule that demonstrates using a cron rule with decorators", tags=["Test tag", "Hello World"])# [description and tags are optional]
@when("Time cron 0/10 * * * * ?")
def hellowWorldDecoratorCron(event):
    from logging import DEBUG, INFO, WARNING, ERROR
    hellowWorldDecoratorCron.log.setLevel(DEBUG)
    hellowWorldDecoratorCron.log.info("This is a 'hello world!' from a Jython rule (decorator): Cron")

OK, today no progress for me really, just learning ...

EDIT:

In lucid (rules.py) I had the following code:

from org.eclipse.smarthome.core.library.types import IncreaseDecreaseType, NextPreviousType, OnOffType, OpenClosedType, PlayPauseType, RewindFastforwardType, StopMoveType, UpDownType, DecimalType

from lucid.log import logging, log_traceback, LOG_PREFIX
from logging import DEBUG, INFO, WARNING, ERROR

import lucid.config as config

from lucid.jsr223 import scope, get_automation_manager
from lucid.jsr223.scope import events, itemRegistry
import random
import time

NULL = UnDefType.NULL
UNDEF = UnDefType.UNDEF
ON = OnOffType.ON
OFF = OnOffType.OFF
OPEN = OpenClosedType.OPEN
CLOSED = OpenClosedType.CLOSED

and

def wrap_execute(fn):
    """Wrapper to extend the execute function"""
    functools.wraps(fn)
    def wrapper(self, modules, inputs):
        hasReloadFinished()
        self.event = Event(inputs)

        # Add global names, using the function's own global namespace:
        g = fn.__globals__
        g['DEBUG'] = DEBUG
        g['INFO'] = INFO
        g['WARNING'] = WARNING
        g['ERROR'] = ERROR
        g['NULL'] = NULL
        g['UNDEF'] = UNDEF
        g['ON'] = ON
        g['OFF'] = OFF
        g['OPEN'] = OPEN
        g['CLOSED'] = CLOSED

        fn(self, modules, inputs)
        # Place to add stuff after wrapped function
    return wrapper

I added this wrapper function into lucid trying to make just small things easier for people who like to get started scripting without prior experience. Is it something we'd like to retain or what do you think?

[5iver]

I guess that when I'm finished I can make a PR to your repo openhab-5iver/openhab2-jython Do you think that will be a good way forward?

Yes... that was one of the options. 😄 I'll also be getting the gitignore change synced.

DEBUG is no longer in the global scope and even if I try to set the logging level for a rule it doesn't seem to make any difference.

Can you just change this...

hellowWorldDecoratorCron.log.info("This is a 'hello world!' from a Jython rule (decorator): Cron")

to

hellowWorldDecoratorCron.log.debug("This is a 'hello world!' from a Jython rule (decorator): Cron")

Or am I missing something? I have set the LOG_PREFIX to "jsr223.jython", so be sure to set the logging level for this to at least DEBUG. You could log:set DEBUG jsr223.jython, or I manually update the org.ops4j.pax.loging.cfg file with...

log4j2.logger.jython.name = jsr223.jython
log4j2.logger.jython.level = DEBUG

... which is what the Karaf console command is adding. I also setup two appenders... one for rules and one for core. I'll be sure to add some explanation for logging in the doc.

I added this wrapper function into lucid trying to make just small things easier for people who like to get started scripting without prior experience. Is it something we'd like to retain or what do you think?

All of this (and more) is added to the context through the "default" scriptExtension, which is added to all scripts...

from org.eclipse.smarthome.core.library.types import IncreaseDecreaseType, NextPreviousType, OnOffType, OpenClosedType, PlayPauseType, RewindFastforwardType, StopMoveType, UpDownType, DecimalType

NULL = UnDefType.NULL
UNDEF = UnDefType.UNDEF
ON = OnOffType.ON
OFF = OnOffType.OFF
OPEN = OpenClosedType.OPEN
CLOSED = OpenClosedType.CLOSED

... so I did not see the need for them (number 5 in the Things Removed section of OP). Are you not able to access them in your scripts?!

[5iver]

OK... git fought hard, but I finally got things synced. The commits look weird, but I'll squash before the merge.

[besynnerlig]

EDIT: Logging levels worked as before. I removed that part from this text.
Checking my 2.3 installation lucid log level shows DEBUG! That was a surprise. I didn't know I had set it to that. It seems that I can only use setLevel in a rule to a higher value locally than what is specified globally. The other way around doesn't work and probably shouldn't. ;)

I haven't tested a lot but the hello_world.py script can't access the global name DEBUG
NameError: global name 'DEBUG' is not defined . (ON is accessible though)

Cheers!

EDIT: I'm setting up a copy of my home environment for testing purposes. I'll have access to all my familiar OH Items etc that I'm used to and I'll be able to test the migration better. It takes some time though but I feel there is some progress. ;)

[5iver]

I haven't tested a lot but the hello_world.py script can't access the global name DEBUG
NameError: global name 'DEBUG' is not defined . (ON is accessible though)

I'm a little confused... is this not an issue?

[5iver]

@besynnerlig, I've made some more changes that will need your review and synced up with master to pick up the core.date fixes. I did not include the rename of the /automation/jsr223/core/. I will do that right after this PR merged, so just rename your core directory to 000_core for testing.

[besynnerlig]

@besynnerlig, I've made some more changes that will need your review and synced up with master to pick up the core.date fixes. I did not include the rename of the /automation/jsr223/core/. I will do that right after this PR merged, so just rename your core directory to 000_core for testing.

Thanks. I'll have a look at it tomorrow (monday morning).

[besynnerlig]

@besynnerlig, I've made some more changes that will need your review and synced up with master to pick up the core.date fixes. I did not include the rename of the /automation/jsr223/core/. I will do that right after this PR merged, so just rename your core directory to 000_core for testing.

My web repo branch is 23 commits ahead, 9 commits behind openhab-5iver:lucid-migration. I have no idea how to re-sync. I guess the easiest would be to discard my repo on my local machine and clone yours again. I think I'll do just that.

[5iver]

I guess the easiest would be to discard my repo on my local machine and clone yours again. I think I'll do just that.

That would do it, but this should work too (do this from inside the git directory of your clone)...

git checkout lucid-migration
git reset --hard origin/lucid-migration

[besynnerlig]

I have finished testing. I found just a couple of problems that I'll submit to your personal repo as a PR.

When you feel that the time is right, please let me know and I'll make a more extensive test. Then I will migrate my production site from OH 2.3 to OH 2.5M1. That will include migrating all my jython scripts.

EDIT: Made PR 5iver#2

[5iver]

When you feel that the time is right, please let me know and I'll make a more extensive test.

After those last two fixes, I think we are ready to merge this. If we find bugs after the merge, it will be a little more effort to update both branches, but not too much.

After the merge, I'll setup a branch to store the <=2.5M1 code, update master to >2.5M1, and get on with the repo renaming!

[5iver]

@besynnerlig, the last two fixes are merged. Are you ready to approve the PR?

[DONE] O wait... need to remove the gitignore entries for community.

[besynnerlig]

Tested and approved.

[5iver]

The way the repo is currently setup, the person that created the PR can't approve their own PR, and an approval is required before it can be merged. Being an admin, I can do anything, but to be official about it, you should start another review and select 'approve changes'. 😄

[besynnerlig]

Reviewed and approved

[besynnerlig]

The way the repo is currently setup, the person that created the PR can't approve their own PR, and an approval is required before it can be merged. Being an admin, I can do anything, but to be official about it, you should start another review and select 'approve changes'. 😄

I'm not sure. Am I approving changes in the wrong place?

[5iver]

No... you don't have write privileges... I'll need to rethink that. I'll waive the magic wand...