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

Cleaned up some logging and renamed PLM "log links" for consistency #71

Merged
merged 2 commits into from
Feb 16, 2013
Merged

Cleaned up some logging and renamed PLM "log links" for consistency #71

merged 2 commits into from
Feb 16, 2013

Conversation

mstovenour
Copy link
Collaborator

Renamed PLM "show link table to log" as "log links" to be consistent with devices
Improved the amount of information supplied by existing logging messages.
Added "{devicename} completed sync links" message
Changed "PLM" NACK warning so it doesn't reference burned out bulbs which only apply to "device" NACKs.

Renamed PLM "show link table to log" as "log links" to be consistent with devices
Improved the amount of information supplied by existing logging messages.
Added "{devicename} completed sync links" message
Changed "PLM" NACK warning so it doesn't reference burned out bulbs which only apply to "device" NACKs.
@@ -301,7 +301,7 @@ sub generate_voice_commands
$object_string .= "$object_name_v -> tie_event('$object_name->complete_linking_as_responder','complete linking as responder');\n\n";
$object_string .= "$object_name_v -> tie_event('$object_name->initiate_unlinking_as_controller','initiate unlinking');\n\n";
$object_string .= "$object_name_v -> tie_event('$object_name->cancel_linking','cancel linking');\n\n";
$object_string .= "$object_name_v -> tie_event('$object_name->log_alllink_table','show link table to log');\n\n";
$object_string .= "$object_name_v -> tie_event('$object_name->log_alllink_table','log links');\n\n";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you need to change the cmd_state on line 299 as well in order for this to work.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. This shows a flaw in my work flow. I omitted a change that I consider part of i2CS dev where I added a new command. I didn’t realize that I was omitting the renaming of that command as well.

From: krkeegan [mailto:notifications@github.com]
Sent: Monday, February 11, 2013 12:01 AM
To: hollie/misterhouse
Cc: Michael Stovenour
Subject: Re: [misterhouse] Cleaned up some logging and renamed PLM "log links" for consistency (#71)

In lib/Insteon.pm:

@@ -301,7 +301,7 @@ sub generate_voice_commands
$object_string .= "$object_name_v -> tie_event('$object_name->complete_linking_as_responder','complete linking as responder');\n\n";
$object_string .= "$object_name_v -> tie_event('$object_name->initiate_unlinking_as_controller','initiate unlinking');\n\n";
$object_string .= "$object_name_v -> tie_event('$object_name->cancel_linking','cancel linking');\n\n";

  •       $object_string .= "$object_name_v -> tie_event('$object_name->log_alllink_table','show link table to log');\n\n";
    
  •       $object_string .= "$object_name_v -> tie_event('$object_name->log_alllink_table','log links');\n\n";
    

I think you need to change the cmd_state on line 299 as well in order for this to work.


Reply to this email directly or view it on GitHub https://github.com/hollie/misterhouse/pull/71/files#r2957530 ..

https://github.com/notifications/beacon/Nvoh7bM6g31WX2NN_lw41aw6CQu8FlPOA0Yz7ZizF67PUMyIBvwKheDfowXkGvVK.gif

Second try to rename PLM "show link table to log" as "log links" to be consistent with devices
@krkeegan
Copy link
Collaborator

Hollie, I could use some help with this one. For whatever reason, possibly because it is two commits, I cannot merge it automatically.

@hollie
Copy link
Owner

hollie commented Feb 14, 2013

Hi Kevin,

I'll get back to you tonight on this.

@hollie
Copy link
Owner

hollie commented Feb 14, 2013

Hi,

I went ahead and tried to manually merge to see what causes the automatic merging to fail. Apparently, this pull request causes a conflict.

To reproduce:

nessie:misterhouse lieven$ git remote add mstovenour git://github.com/mstovenour/misterhouse.git
nessie:misterhouse lieven$ git fetch mstovenour
nessie:misterhouse lieven$ git merge mstovenour/fix_logging_cleanup
Auto-merging lib/Insteon_PLM.pm
Auto-merging lib/Insteon/BaseInsteon.pm
CONFLICT (content): Merge conflict in lib/Insteon/BaseInsteon.pm
Automatic merge failed; fix conflicts and then commit the result.

The conflict as around line 1513. I think it is because the pull request was not originating from a completely up-to-date BaseInsteon.pm file.

If somebody with Insteon testing capabilities can reproduce the steps above, fix the conflict, test and commit then the pull request can be closed.

mstovenour pushed a commit that referenced this pull request Feb 16, 2013
Cleaned up some logging and renamed PLM "log links" for consistency (#71)
Merge conflict lib/Insteon/BaseInsteon.pm
@mstovenour mstovenour merged commit af63929 into hollie:master Feb 16, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants