-
Notifications
You must be signed in to change notification settings - Fork 130
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
Deprecated Code in Newer Perl Versions #406
Comments
Hi Jared, see the pull request #408 to fix the warnings. Can you test please? Note: I did not fix the lib/site modules. To all: is there anybody aware why we include CPAN modules in our codebase instead of fetching them from CPAN? I imagine this is a choice that was made before cpan/cpanm was widely available to ease the installation. Maybe this is no longer required? According to me adding standard modules to our codebase adds much unneeded maintenance work to keep the modules up to date. cpan or cpanm can perfectly handle that for us? |
Hi Lieven, Thanks for the pull request; it looks pretty good to me and all of the warning messages are gone except for the one related to the external module. However, I am seeing a crash when MH attempts to add the hop count to the hop_array for a device (this occurs at startup). I am not really sure why this is happening, because the change you made looks correct to me; I don't see why !defined(@array) and !(@array) should produce a different result... Here is the log output: 05/11/2014 17:49:27 [Insteon_PLM] DEBUG2: Sending obj=$Living_Room_Aquarium_Light; command=get_engine_version incurred delay of 0.28 seconds; starting hop-count: 2 05/11/2014 17:49:27 [Insteon_PLM] DEBUG3: Received PLM raw data: 026214XX790a0d0006 05/11/2014 17:49:27 [Insteon_PLM] DEBUG3: Received PLM acknowledge: obj=$Living_Room_Aquarium_Light; command=get_engine_version 05/11/2014 17:49:27 [Insteon::BaseInterface] DEBUG3: PLM command:insteon_received; Device command:get_engine_version; type:direct; group: As for packing the external modules into MH, I agree that it was likely originally for portability and ease of installation. As long as the modules in question are available through CPAN and will successfully install on most operating systems, I think it would be a good idea to separate them from the MH codebase. For now, would simply updating this module through CPAN (or apt for Debian/Ubuntu) be good enough to fix this issue or will a larger change be necessary for MH/perl to find and use the newer versions of the modules? As a side note, when I upgraded to 14.04, I had to also install the perl Switch module using "apt-get install libswitch-perl" before MH would even start back up. |
Hey @JaredF I added another condition to the if statement that first verifies if $$self{hop_count} exists. Maybe this condition alone is enough to obtain what the original author of the code had in mind, but I left the !(@{$$self{hop_count}}) condition in place too just to be sure. Can you please retry? Concerning the module and installation through CPAN: yes, according to me you can install the module through CPAN and remove the file under lib/site to ensure your installation uses the CPAN one. When you startup MisterHouse the 'module search path' is printed and the location where your Perl installation that you use to run MisterHouse will be included there. That is also the location where the module will be installed if you do it through CPAN. Switch module: OK, thanks. If we decide to remove the CPAN modules from the codebase we will need to make a list of required modules that need to be installed to get MisterHouse up and running. We could add that in an installer script. |
This one also crashes, but with a different error: Not a SCALAR reference at /usr/local/bin/misterhouse/mh/bin/../lib/Insteon/BaseInsteon.pm line 278. I have have tested a few more similar iterations and have found the following to work: if ($$self{hop_array} && !(@{$$self{hop_array}})) { This is a somewhat complicated piece of code for me, so please review and confirm that it performs the same test as the original. From reading online it appears that the 'defined' function doesn't autovivify so perhaps that's the reason that when using 'defined,' the extra test is not required. |
Also confirmed that by removing the HTML folder under lib/site/ and installing 'libhtml-format-perl' in Ubuntu (was already installed on my device) the UNIVERSAL->import warning has disappeared. |
@JaredF I have merged your patch into the pull request. I think we need to discuss further on how to proceed on the module question on the mailing list before we actually remove a module from the codebase, so I will not include that change in the pull request that fixes the deprecated warnings. |
The pull request #408 fixes this. We're still discussing on how to proceed on the CPAN modules on the mailing list. Closing this ticket for now. |
Newer versions of Perl, for example, version 5.18.2 that is distributed with the latest version of Ubuntu (14.04 LTS - Trusty) have deprecated some of the older features of Perl. During startup using Perl 5.18.2, the following errors are thrown:
defined(@array) is deprecated at /usr/local/bin/misterhouse/mh/bin/../lib/Generic_Item.pm line 904.
(Maybe you should just omit the defined()?)
UNIVERSAL->import is deprecated and will be removed in a future perl at ../lib/site/HTML/Formatter.pm line 49.
defined(@array) is deprecated at /usr/local/bin/misterhouse/mh/bin/../lib/Insteon/BaseInsteon.pm line 278.
(Maybe you should just omit the defined()?)
There may be other instances of these functions or even others that have been deprecated as well, these are just the only ones that show for me during startup.
The text was updated successfully, but these errors were encountered: