New scheduling code, WeatherNOAA fixes, and documentation updates for DOORBIRD.pm and HARMON.pm. #629

waynieack · 2016-10-14T02:59:02Z

Added SCHEDULE module for scheduling object state changes via the web UI. This is the backend code for Howard’s ia7 web code. There are 2 sub modules SCHEDULE_Temp and SCHEDULE_Generic.
SCHEDULE_Temp is for thermostat scheduling.
• It allows users to control any thermostat connected to Misterhouse.
• It has objects for setting the thermostat temperatures for each schedule in the MH web UI.
• It has built in occupancy tracking for changing the thermostat temperature settings based on the time and if someone is home or not, including a vacation mode.
• It has a built in outdoor temperature/forecast tracking to change the thermostat temperature settings based on the time and the forecasted temperature or the current outdoor temperature.

SCHEDULE_Generic is for scheduling object state changes for objects that don’t inherit the scheduling code from the Generic_Item.

Generic_Item now has the scheduling code as well so all Generic_Item objects or objects that inherit Generic_Item will be able to have state changes scheduled via Howard's scheduling web UI code. The schedule check code is not added to the main loop until the user adds a schedule via the web UI and the code is removed from the main loop once all the schedule entries are removed.

get_weather has the fixes from Rick Steeves.
WeatherNOAA has fixes from Rick and I.

DOORBIRD.pm and HARMON.pm now have documentation that converts to HTML properly and can be viewed in the local Misterhouse POD documentation page.

DOORBIRD.pm has been updated to post back to Misterhouse using the no_response option.

_Wayne

marcmerlin

Thanks for the fixes, a few cleanups needed.

But more generally this is not a really proper way to send a review, you're sending us changes, then reverts, and then more changes on top, making the review very hard.

More generally, there are 3 different changes totally unrelated, in this CL.
I realize it's a bit of a pain from your side, but the normal way is to make a branch for each change and send one combined change (no multiple changes and reverts and more patches) in each pull.

Now I realize this may be a lot of work here.
If it's prohibitive, try to clean up your current change and collapse it as one diff, and I'll try and do a review of all 3, even if they are unrelated and shouldn't be combined in one pull, but please avoid this in the future.

In the meantime, I manually merged just your weather changes to my tree and will see if they fix my problems too. Thanks again for those.

marcmerlin · 2016-10-19T04:08:42Z

bin/get_weather

-
-    # Dang ... a 'do pgm' can only return a scalar, not a list :(
-    if ( $data{conditions} ) {
+                                # Dang ... a 'do pgm' can only return a scalar, not a list :(


formatting broken

marcmerlin · 2016-10-19T04:08:59Z

bin/get_weather

-    if ( $data{conditions} ) {
-        print "\nCurrent conditions: $conditions\n";
+    if ($data{conditions}) {
+        #print "\nCurrent conditions: $conditions\n";


why is this commented out?

marcmerlin · 2016-10-19T04:09:49Z

bin/get_weather

+    # RICK 160817
+    #print "Getting the forecast for $parms{zone}, $parms{state}\n";
+    print "Getting the forecast for $parms{city}, $parms{state}, $parms{zone}\n";
+    #$forecast = print_forecast($parms{zone}, $parms{state},undef,undef,undef,1);


please delete commented code

marcmerlin · 2016-10-19T04:09:59Z

bin/get_weather

-    $forecast =~ s/Geo::WeatherNOAA.pm .+\n//;    # Drop geo version
-                                                  #$forecast =~ s/\.\.\./\. /g;
+if ($data{forecast}) {
+    # RICK 160817


please delete

marcmerlin · 2016-10-19T04:10:28Z

bin/get_weather

-      print_current( $parms{city}, $parms{state}, undef, undef, undef, 1 );
+if ($data{conditions}) {
+    # Rick 160816
+    #print "\nGetting the current weather for $parms{city}, $parms{state}\n";


please delete commented lines

marcmerlin · 2016-10-19T04:11:00Z

bin/get_weather

 my %config_parms;

 $parms{city}  = 'Rochester'  unless $parms{city};
-$parms{zone}  = $parms{city} unless $parms{zone};
+#RICK 160817
+#$parms{zone}  = $parms{city} unless $parms{zone};


please delete commented lines

marcmerlin · 2016-10-19T04:31:00Z

@hollie could you also confirm that the diffs look all bad (lots of unchanged code with simply formatting changes) because @waynieack didn't run a perl reformatter on his changes, or is it another problem?

marcmerlin · 2016-10-19T13:21:59Z

@waynieack I tried to install just your fixes to weather in my tree:
wget --no-check-certificate https://raw.githubusercontent.com/waynieack/misterhouse/9cedc478cf1ca56f2db4ab898411f97be920395f/bin/get_weather
wget --no-check-certificate https://raw.githubusercontent.com/waynieack/misterhouse/9cedc478cf1ca56f2db4ab898411f97be920395f/lib/site/Geo/WeatherNOAA.pm

but I still don't seem to be getting weather with this, am I supposed to change the way I call it?
gargamel:/var/local/src/misterhouse# mh/bin/get_weather -zone "SAN JOSE" -city "SAN JOSE" -state CA

Getting the current weather for SAN JOSE, CA, SAN JOSE
Getting the forecast for SAN JOSE, CA, SAN JOSE
gargamel:/var/local/src/misterhouse# cat /var/local/src/misterhouse/data/web/weather_forecast.txt
As of :
gargamel:/var/local/src/misterhouse#

hollie · 2016-10-19T17:29:19Z

@waynieack first of all thanks for this pull request!

As @marcmerlin mentions: normally it is indeed the idea to split up different fixes or adaptations into different pull requests because this makes the code easier to review. You can do this by making a new branch in your working copy, as described here: https://github.com/hollie/misterhouse/wiki/Contributing#developing-using-git

Then a general remark: you don't have to comment out lines and add your name in front. Since we're using git we know exactly who is responsible for a specific code change ;-)

There are indeed a few changes that are only related to whitespaces that are changed. Did you run a specific formatter on the code to do that? We have a specific set of Perltidy settings that we use to format the code in this project. For more info please see here: https://github.com/hollie/misterhouse/wiki/Contributing#using-perltidy-to-make-consistent-formatted-code

I think that there is lots of interesting code in this pull request, so if you could update the changes requested by Marc and rerun the formatting on your code then we'll look into integrating it into master.

Note: if you update your branch this pull request should also receive the updates. This is one of the reasons you should normally not create a pull request from your master branch.

waynieack · 2016-10-19T21:20:08Z

Sorry, I'm new to git. I had some issues with conflicts and made a mess
with the reverts. For get_weather, Steve worked on that and emailed it to
me, I'll clean it up, (figure out how to make a new branch:)) make a branch
and create a new pull request.

The changes to WeatherNOAA.pm were all related to this issue, is that OK to
do in a single branch?

Do you want me to cancel the current pull request?

_Wayne

On Tue, Oct 18, 2016 at 11:16 PM, Marc MERLIN [email protected]
wrote:

@marcmerlin requested changes on this pull request.

Thanks for the fixes, a few cleanups needed

But more generally this is not a really proper way to send a review,
you're sending us changes, then reverts, and then more changes on top,
making the review very hard.

More generally, there are 3 different changes totally unrelated, in this
CL.
I realize it's a bit of a pain from your side, but the normal way is to
make a branch for each change and send one combined change (no multiple
changes and reverts and more patches) in each pull.

Now I realize this may be a lot of work here.
If it's prohibitive, try to clean up your current change and collapse it
as one diff, and I'll try and do a review of all 3, even if they are
unrelated and shouldn't be combined in one pull, but please avoid this in

the future.

In bin/get_weather
#629 (review)
:

}

if ($return_flag) {
Dang ... a 'do pgm' can only return a scalar, not a list :(

if ( $data{conditions} ) {
                           # Dang ... a 'do pgm' can only return a scalar, not a list :(
formatting broken

In bin/get_weather
#629 (review)
:
     return $conditions;
 }
 else {
     return $forecast;
 }
}
else {
if ( $data{conditions} ) {
   print "\nCurrent conditions: $conditions\n";
if ($data{conditions}) {
   #print "\nCurrent conditions: $conditions\n";
why is this commented out?

In bin/get_weather
#629 (review)
:
 $conditions =~ s/&deg;F/ degrees /;
 $conditions =~ s/ in\./ inches. /g;
}
-if ( $data{forecast} ) {
print "Getting the forecast for $parms{zone}, $parms{state}\n";

$forecast =
 print_forecast( $parms{zone}, $parms{state}, undef, undef, undef, 1 );
$forecast =~ s/Geo::WeatherNOAA.pm .+\n//; # Drop geo version
                                             #$forecast =~ s/.../. /g;
+if ($data{forecast}) {
RICK 160817

#print "Getting the forecast for $parms{zone}, $parms{state}\n";

print "Getting the forecast for $parms{city}, $parms{state}, $parms{zone}\n";

#$forecast = print_forecast($parms{zone}, $parms{state},undef,undef,undef,1);
please delete commented code

In bin/get_weather
#629 (review)
:
 $conditions =~ s/&deg;F/ degrees /;
 $conditions =~ s/ in\./ inches. /g;
}
-if ( $data{forecast} ) {
print "Getting the forecast for $parms{zone}, $parms{state}\n";

$forecast =
 print_forecast( $parms{zone}, $parms{state}, undef, undef, undef, 1 );
$forecast =~ s/Geo::WeatherNOAA.pm .+\n//; # Drop geo version
                                             #$forecast =~ s/.../. /g;
+if ($data{forecast}) {
RICK 160817
please delete

In bin/get_weather
#629 (review)
:

use Geo::WeatherNOAA;

$Geo::WeatherNOAA::proxy_from_env = 1;

-if ( $data{conditions} ) {
print "\nGetting the current weather for $parms{city}, $parms{state}\n";

$conditions =
 print_current( $parms{city}, $parms{state}, undef, undef, undef, 1 );
+if ($data{conditions}) {
Rick 160816

#print "\nGetting the current weather for $parms{city}, $parms{state}\n";
please delete commented lines

In bin/get_weather
#629 (review)
:

my %config_parms;

$parms{city} = 'Rochester' unless $parms{city};
-$parms{zone} = $parms{city} unless $parms{zone};
+#RICK 160817
+#$parms{zone} = $parms{city} unless $parms{zone};

please delete commented lines

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#629 (review),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AFZ3eRDcWeQ983MbxYZI6vo-ynSXwJusks5q1ZmUgaJpZM4KWlwh
.

marcmerlin · 2016-10-19T21:56:15Z

anwers

you don't need one pull request per file, but per logical change. So in your case, you should put both weather files in the same pull request (also each pull request should create a tree that is consistent and works after installing just that pull request)
in this case, you may want to take a clean tree on your side, make a branch, copy your weather changes in it, make a pull request for just that.
Then do another tree with another branch with a different name and put your unrelated pull request with related new features in that one.
After you upload those 2 and they look good online, you can delete this one.
remember to run perltidy so that you don't send diffs that undo the current white space rules and make no other code changes

waynieack · 2016-10-19T23:47:45Z

The zone is a 3 letter code from:
http://forecast.weather.gov/product_sites.php?site=CRH&product=ZFP.

The easiest way I've found to get the zone is to search your zip or
city/state in the "Local forecast by" search box at the top left, then get
the 3 letter zone from the URL you are redirected to. (the zone is after
"&site=" in the URL)

If I search Baton Rouge, LA I get redirected to
http://forecast.weather.gov/MapClick.php?CityName=Baton+Rouge&state=LA&site=LIX&lat=30.449&lon=-91.126#.WAgBXiSPFv0

So my zone is: LIX

San Jose is: MTR

Call it like this:
get_weather -zone MTR -city "SAN JOSE" -state CA

It looks like I can hack together a way to get the zone with the city and
state using a post to the search box and pulling the zone from the site=
field of the location header. The only issue with this is that if any
changes are made to the page it will break, so I don't know if its worth
it. What do you think?

If we go this route then get_weather and the user mh.private.ini would not
be changed and I could just update WeatherNOAA.pm.

_Wayne

On Wed, Oct 19, 2016 at 8:22 AM, Marc MERLIN [email protected]
wrote:

@waynieack https://github.com/waynieack I tried to install just your
fixes to weather in my tree:
wget --no-check-certificate https://raw.githubusercontent.
com/waynieack/misterhouse/9cedc478cf1ca56f2db4ab898411f9
7be920395f/bin/get_weather
wget --no-check-certificate https://raw.githubusercontent.
com/waynieack/misterhouse/9cedc478cf1ca56f2db4ab898411f9
7be920395f/lib/site/Geo/WeatherNOAA.pm

but I still don't seem to be getting weather with this, am I supposed to
change the way I call it?
gargamel:/var/local/src/misterhouse# mh/bin/get_weather -zone "SAN JOSE"
-city "SAN JOSE" -state CA

Getting the current weather for SAN JOSE, CA, SAN JOSE
Getting the forecast for SAN JOSE, CA, SAN JOSE
gargamel:/var/local/src/misterhouse# cat /var/local/src/misterhouse/
data/web/weather_forecast.txt
As of :
gargamel:/var/local/src/misterhouse#

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#629 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AFZ3eXlEKBYf0aJkGIiiOuW7Z9lGrgIBks5q1hl6gaJpZM4KWlwh
.

waynieack · 2016-10-20T03:01:08Z

Just to be sure I don't make more of a mess than I already have :)

I set up the hook with perl tidy so all my commits will run through it now.

Copy my changed files out.
Reset all the changes from the last commit
git reset --hard b670e15
git commit -am "Revert to move to changes to branches"

git checkout -b Schedule

copy SCHEDULE.pm and Generic_Item.pm back

git stage SCHEDULE.pm
git stage Generic_Item.pm
git commit -m "Completed SCHEDULE.pm beta, adding more options, some bug
fixes, and Documentation. Added scheduling code in Generic_Item, so all
Generic_Item objects or objects that inherit Generic_Item will have
scheduling options in the UI."
git push origin Schedule

git checkout -b Harmon

copy HARMON.pm back

git stage HARMON.pm
git commit -m "Updated Documentation"
git push origin Harmon

git checkout -b Doorbird

copy DOORBIRD.pm back

git stage DOORBIRD.pm
git commit -m "Updated Documentation"
git push origin Doorbird

Fix issues with get_weather noted in current pull request.

git checkout -b WeatherNOAA

copy WeatherNOAA.pm and get_weather back

git stage WeatherNOAA.pm
git stage get_weather
git commit -m "Fixes for broken code due to NOAA site changes. Bug #631"
git push origin WeatherNOAA

cancel current pull request
create a pull request for each of the above.

Let me know if I am on the right track.

_Wayne

On Wed, Oct 19, 2016 at 4:56 PM, Marc MERLIN [email protected]
wrote:

anwers

you don't need one pull request per file, but per logical change. So in
your case, you should put both weather files in the same pull request (also
each pull request should create a tree that is consistent and works after
installing just that pull request)

in this case, you may want to take a clean tree on your side, make a
branch, copy your weather changes in it, make a pull request for just that.
Then do another tree with another branch with a different name and put
your unrelated pull request with related new features in that one.
After you upload those 2 and they look good online, you can delete this
one.

remember to run perltidy so that you don't send diffs that undo the
current white space rules and make no other code changes

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#629 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AFZ3efBNp5uF5uzzxpRaoInKX98YKoVeks5q1pICgaJpZM4KWlwh
.

marcmerlin · 2016-10-20T03:24:56Z

First, I missed the part that I have to give new arguments to get_weather, it works now, thank you.
As for how to do the changes, looking at what you're saying, I think it'll work, but ultimately you'll see what it looks like in your github tree when you upload.
You can see if that diff looks clean and self contained before you send a pull request to this tree. Github makes it easy to see what you're sending before you send it.

Master merge

This reverts commit 9dac309.

…ded when get_weather is called. Bug hollie#631

Updated Harmon Documentation

Doorbird merge

Fixes for broken code due to NOAA site changes. Bug hollie#631

waynieack · 2016-10-25T18:18:02Z

I made 3 new pull requests, the Harmon and Doorbird requests failed some of
the checks but I am not sure why. The code did not change and the checks
passes on the first pull request.

I did however run all the code through perl tidy with the hook, so there
are many changes in the diffs due to that.

I updated the weather NOAA code so the zone is not needed anymore.

Fixes for broken code due to NOAA site changes. Bug #631
#635
#635 (comment)
#635

Doorbird documentation updates.
#634
#634 (comment)
#634

Harmon documentation updates
#633
#633 (comment)
#633

_Wayne

On Wed, Oct 19, 2016 at 10:24 PM, Marc MERLIN [email protected]
wrote:

First, I missed the part that I have to give new arguments to get_weather,
it works now, thank you.
As for how to do the changes, looking at what you're saying, I think it'll
work, but ultimately you'll see what it looks like in your github tree when
you upload.
You can see if that diff looks clean and self contained before you send a
pull request to this tree. Github makes it easy to see what you're sending
before you send it.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#629 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AFZ3eafmWthhQShAqWBi-Va97ftqHOiGks5q1t8LgaJpZM4KWlwh
.

hollie · 2016-10-25T20:48:44Z

Hello Wayne,

regarding the tests, yes I'm aware of the problem and I'm looking into it. Might take until the weekend before I find some time to clear that out.

Thanks for splitting the contributions in separate pull requests.

Best regards,
Lieven.

Added alpha SCHEDULE module

b670e15

waynieack changed the title ~~WeatherNOAA fixes, and documentation updates for DOORBIRD.pm and HARMON.pm.~~ New scheduling code, WeatherNOAA fixes, and documentation updates for DOORBIRD.pm and HARMON.pm. Oct 14, 2016

marcmerlin requested changes Oct 19, 2016

View reviewed changes

Revert to move to changes to branches

886e411

waynieack force-pushed the master branch from 0241e73 to 886e411 Compare October 20, 2016 20:10

Wayne Gatlin added 5 commits October 20, 2016 15:15

Merge pull request #2 from hollie/master

31eb7ed

Master merge

Updated Harmon Documentation

bb0a6a8

Updated Doorbird Documentation

9dac309

Revert "Updated Doorbird Documentation"

d8ad966

This reverts commit 9dac309.

Updated Doorbird Documentation

25f79ee

marcmerlin mentioned this pull request Oct 21, 2016

NOAA website changes broke lib/site/Geo/WeatherNOAA.pm and get_weather #631

Closed

waynieack added 4 commits October 21, 2016 21:36

Fixes for broken code due to NOAA site changes. Zone is no longer nee…

0226bad

…ded when get_weather is called. Bug hollie#631

Merge pull request #3 from waynieack/Harmon

8d373be

Updated Harmon Documentation

Merge pull request #4 from waynieack/Doorbird

b9cfac0

Doorbird merge

Merge pull request #5 from waynieack/WeatherNOAA

053bdef

Fixes for broken code due to NOAA site changes. Bug hollie#631

waynieack closed this Oct 22, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New scheduling code, WeatherNOAA fixes, and documentation updates for DOORBIRD.pm and HARMON.pm. #629

New scheduling code, WeatherNOAA fixes, and documentation updates for DOORBIRD.pm and HARMON.pm. #629

waynieack commented Oct 14, 2016 •

edited

Loading

marcmerlin left a comment •

edited

Loading

marcmerlin Oct 19, 2016

marcmerlin Oct 19, 2016

marcmerlin Oct 19, 2016

marcmerlin Oct 19, 2016

marcmerlin Oct 19, 2016

marcmerlin Oct 19, 2016

marcmerlin commented Oct 19, 2016

marcmerlin commented Oct 19, 2016

hollie commented Oct 19, 2016

waynieack commented Oct 19, 2016

the future.

if ($return_flag) {

Dang ... a 'do pgm' can only return a scalar, not a list :(

formatting broken

why is this commented out?

RICK 160817

please delete commented code

RICK 160817

please delete

Rick 160816

please delete commented lines

marcmerlin commented Oct 19, 2016

waynieack commented Oct 19, 2016

waynieack commented Oct 20, 2016

marcmerlin commented Oct 20, 2016

waynieack commented Oct 25, 2016

hollie commented Oct 25, 2016

New scheduling code, WeatherNOAA fixes, and documentation updates for DOORBIRD.pm and HARMON.pm. #629

New scheduling code, WeatherNOAA fixes, and documentation updates for DOORBIRD.pm and HARMON.pm. #629

Conversation

waynieack commented Oct 14, 2016 • edited Loading

marcmerlin left a comment • edited Loading

Choose a reason for hiding this comment

marcmerlin Oct 19, 2016

Choose a reason for hiding this comment

marcmerlin Oct 19, 2016

Choose a reason for hiding this comment

marcmerlin Oct 19, 2016

Choose a reason for hiding this comment

marcmerlin Oct 19, 2016

Choose a reason for hiding this comment

marcmerlin Oct 19, 2016

Choose a reason for hiding this comment

marcmerlin Oct 19, 2016

Choose a reason for hiding this comment

marcmerlin commented Oct 19, 2016

marcmerlin commented Oct 19, 2016

hollie commented Oct 19, 2016

waynieack commented Oct 19, 2016

the future.

if ($return_flag) {

Dang ... a 'do pgm' can only return a scalar, not a list :(

formatting broken

why is this commented out?

RICK 160817

please delete commented code

RICK 160817

please delete

Rick 160816

please delete commented lines

marcmerlin commented Oct 19, 2016

waynieack commented Oct 19, 2016

waynieack commented Oct 20, 2016

marcmerlin commented Oct 20, 2016

waynieack commented Oct 25, 2016

Fixes for broken code due to NOAA site changes. Bug #631 #635 #635 (comment) #635

Doorbird documentation updates. #634 #634 (comment) #634

hollie commented Oct 25, 2016

waynieack commented Oct 14, 2016 •

edited

Loading

marcmerlin left a comment •

edited

Loading

Fixes for broken code due to NOAA site changes. Bug #631
#635
#635 (comment)
#635

Doorbird documentation updates.
#634
#634 (comment)
#634