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

Fix issue 139: lib/Insteon/ file fixes #140

Merged
merged 4 commits into from
Jun 1, 2013
Merged

Conversation

jame
Copy link
Contributor

@jame jame commented Mar 24, 2013

  1. Add a shabang line to MessageDecoder_test.pl like this: "#!/usr/bin/perl -w".
  2. Remove the executable mode setting from the *.pm files.

@mstovenour
Copy link
Collaborator

Did you test the decoder after making this change? I'm not sure about the -w. "should" be fine but needs to be tested.

@jame
Copy link
Contributor Author

jame commented Mar 25, 2013

The same issues come up using the warnings pragma whether the test script is run at the command line like this:

perl -w MessageDecoder_test.pl

as it does running with something like this as the first line.

#!/usr/bin/perl -w

So no, there appears to be no difference in the scripts output.

@jame
Copy link
Contributor Author

jame commented Mar 25, 2013

Note that what that is coming up with on my system (Debian Wheezy) are multiple instances of the following:

Use of uninitialized value in concatenation (.) or string at ../Insteon/MessageDecoder.pm line 520.
Use of uninitialized value in multiplication (*) at ../Insteon/MessageDecoder.pm line 521.
Use of uninitialized value in multiplication (*) at ../Insteon/MessageDecoder.pm line 524.

Mostly of the latter two. If someone else hasn't had a chance to look at those, I could take a look...

@mstovenour
Copy link
Collaborator

you get those errors when using MH with Insteon:4 debug level or you get those errors when using MessageDecoder_test.pl? And only when you have the -w or does this occur w/o the -w option? I use a slightly older Debian distribution (as well as Windows 7 / Activestate Perl 5.12) and have not seen those errors. Maybe it is specific to the message you are decoding? If so can you show me that message's hex data?

@jame
Copy link
Contributor Author

jame commented Mar 27, 2013

They show up when running MessageDecoder_test.pl, and when the warnings pragma is being used.

@jame
Copy link
Contributor Author

jame commented Mar 28, 2013

Note that I think that the issues that using the warnings pragma is finding in MessageDecoder.pm is separate from what I originally intended #139 to address and #140 to fix. If it's not actually on someones todo list already (and I figured it was since I also assume by default that the warnings pragma is being used for public code) I could open a new bug for it and include the results I've found.

@jame
Copy link
Contributor Author

jame commented Mar 28, 2013

Same issues result from using the warnings pragma when running the MessageDecoder_test.pl script on Debian v6.05 system, which has Perl v5.10.
If you also run it in the same way, I would be surprised if the same issues with the module does not come up.

@mstovenour
Copy link
Collaborator

I don't think there is a culture of using warnings in the misterhouse project. I would be surprised if mh its self will run cleanly with warnings turned on. That being said I agree that something as standalone as the message decoder "could" be cleaned up to run with warnings enabled. I'll work on it when I get time. In the mean while you could issue a new pull request w/o the -w otherwise I think this pull request should wait until I get time to fix the warnings.

@jame
Copy link
Contributor Author

jame commented Mar 30, 2013

I don't see a problem with this pull request being held until the module has been fixed. I'll open a separate bug for the Issues found in the Insteon/MessageDecoder.pm module.

@krkeegan krkeegan merged commit ddf6f2f into hollie:master Jun 1, 2013
@krkeegan
Copy link
Collaborator

krkeegan commented Jun 1, 2013

Merged in as #208

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