Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Issues with use as a library #144

Open
kizniche opened this issue Oct 27, 2024 · 14 comments
Open

Issues with use as a library #144

kizniche opened this issue Oct 27, 2024 · 14 comments

Comments

@kizniche
Copy link
Contributor

kizniche commented Oct 27, 2024

Is there a way to run the calibration method without influxdb?

When I originally made the pull request to turn this into a class (#62) for a Mycodo Input (https://github.com/kizniche/Mycodo/blob/e5be39b864702c93b0f72ff0438de504f898bb72/mycodo/inputs/power_monitor_rpi.py, https://github.com/kizniche/rpi-power-monitor/) and other software that wants to use it as as library, calibration could be performed in the terminal without influxdb. Now it seems it exits if terminal mode is used and it can't conenct to influxdb. This presents an issue, since Mycodo uses influxdb 2.x by default and you can't have 1.8 and 2.x installed at the same time. Additionally, it would be a hassle to do that just for a simple calibration. Perhaps a non-database calibration argument can be added for applications where influx 1.8 can't be installed or isn't desired?

@kizniche
Copy link
Contributor Author

Now that I'm digging into the code, I'm seeing there's no way to load a configuration that doesn't include a config file. Previously, this could be acomplished by passing config variables to the RPiPowerMonitor class at initialization. This poses another issue, since using the RPiPowerMonitor in a pure Python application, like Mycodo, there's no config file, since the application itself stores the relevant config variables and needs to be able to pass them to the class at initialization. I haven't looked into the latest version of the code extensively, so I may be wrong... Is there a way to pass configuration variables to the class rather than using a file?

@kizniche
Copy link
Contributor Author

As I look into it more, it seems v0.3 is not backwards compatible with v0.2. If influxdb were made optional and configuration variables could be passed to the class, this would make it able to be used as a library for other software. Otherwise, I'm afraid I won't be able to upgrade the Mycodo Input to use beyond v0.2.

@kizniche kizniche changed the title Calibrate without influxdb? Issues with use as a library Oct 27, 2024
@David00
Copy link
Owner

David00 commented Oct 28, 2024

Hey @kizniche, I'm sure this can be done - I'll take a look at it soon.

@kizniche
Copy link
Contributor Author

kizniche commented Oct 28, 2024

Thanks. I was going to take a crack at it when I have a few good hours I can sit down and test. Are there any significant issues with using v0.2, accuracy-wise I should know about? I have a project I'm currently using the v0.2 in Mycodo and am just curious if there were any major changes to 0.3 that corrected any measurement/calculation issues.

@David00
Copy link
Owner

David00 commented Oct 29, 2024

Accuracy-wise, v0.3 should be slightly better because the delay in time between the voltage and current samples is reduced. However, the difference is probably negligible. The changes were not to correct any issues per-se; I just realized that the way I had originally written it made the original complicated calibration process a hard requirement to even use the power monitor.

With v0.3, the calibration process is vastly simplified to a simple spot-check of the amperage reading of each CT against a trusted handheld amperage meter. The downside is that anyone coming from v0.2 can't simply move their calibration values over to v0.3 as the specific values are meaningless in v0.3. Hopefully that makes sense.

@David00
Copy link
Owner

David00 commented Nov 24, 2024

Hey @kizniche, is there a standard interface that Mycodo relies on to set configuration options? Or, is it kind of custom for each external plugin?

I would be leaning towards accepting a dictionary object containing the configuration settings in the class constructor because this most closely resembles how the config is loaded from the TOML file that standalone users setup.

@kizniche
Copy link
Contributor Author

There's no standard, so whatever works best for your library will do and I'll update the Mycodo Input Module to use that method. A dictionary would do just fine.

@David00
Copy link
Owner

David00 commented Jan 28, 2025

Hey @kizniche! I have finally gotten around to revamping some things, and the RPiPowerMonitor class can now be imported and configured without reliance on a config file. I've also added the beginnings of a simple user level API for the RPiPowerMonitor class. Both of these new features need documentation, which I'll be adding on as I get closer to merging this to master.

I am working off my develop-v0.4.0 branch, and I've just pushed some updates, including a sample script that imports the RPiPowerMonitor class.

The import_example.py script has the full config dictionary listed as well, though all of the pre-v0.4.0 options mirror the config values in my docs:
https://david00.github.io/rpi-power-monitor/docs/v0.3.0/configuration.html#configuration-reference-manual

Note that you'll need python3-dev installed to be able to install the power monitor package. I'd recommend installing like this:

python3 -m venv venv
git clone https://github.com/David00/rpi-power-monitor.git ./rpi_power_monitor
cd rpi_power_monitor
git checkout develop-v0.4.0
source ~/venv/bin/activate
pip install .

Tnen, you can run the import_example.py script with:

python3 ~/rpi_power_monitor/rpi_power_monitor/import_example.py 

As long as your Pi has SPI enabled, it should work!

@kizniche
Copy link
Contributor Author

Thanks for the update! I'll check it out and see if I notice any issues importing and using it in the Mycodo module.

@kizniche
Copy link
Contributor Author

kizniche commented Feb 2, 2025

In the middle of updating my module, I came across something I was wondering if you could clarify. In the previous version, there would be returned one voltage, then the power, current, and pf for each channel. This version returns a voltage for each channel. Could you help me understand why the voltage is now returned with each channel rather than once? Thanks,.

@kizniche
Copy link
Contributor Author

kizniche commented Feb 2, 2025

Also, could I request you also use ">=" for the influxdb dependency, like is used for influxdb-client, so it doesn't overwrite the version already installed? Currently, it will downgrade influxdb if there happens to be a newer version installed.

"influxdb==5.3.2",
"influxdb-client>=1.48.0",

@David00
Copy link
Owner

David00 commented Feb 3, 2025

In the middle of updating my module, I came across something I was wondering if you could clarify. In the previous version, there would be returned one voltage, then the power, current, and pf for each channel. This version returns a voltage for each channel. Could you help me understand why the voltage is now returned with each channel rather than once? Thanks,.

Hi @kizniche, great question! The multiple voltages are essentially irrelevant, but I can understand where your question is coming from. The short answer is that any of them can be used as the voltage value.

The long answer is because each CT input has its own relative AC voltage wave constructed for the calculations. This minimizes the phase error in calculations between channels.

When running the RPiPowerMonitor's run_main function, the voltage measurement that gets stored in the database is really just taken from the first enabled channel's calculation.

voltage = results[self.enabled_channels[0]]['voltage']

IMO, the in-progress user level API functions for the RPiPowerMonitor should abstract this detail to make it easier to use. I will aim to remove the voltage key from each CT, and simply put one voltage key at the top level of the returned dict from the user-level API functions (get_power_measurements and get_single_channel_measurements), like this:

# Sample Response (from a PCB with no sensors attached)
    '''
    
    {1: {'type': 'consumption', 'power': 0, 'current': 0, 'pf': 0},
     2: {'type': 'consumption', 'power': 0, 'current': 0, 'pf': 0},
     3: {'type': 'consumption', 'power': 0, 'current': 0, 'pf': 0},
     4: {'type': 'consumption', 'power': 0, 'current': 0, 'pf': 0},
     5: {'type': 'consumption', 'power': 0, 'current': 0, 'pf': 0},
     6: {'type': 'consumption', 'power': 0, 'current': 0, 'pf': 0},
     'voltage' : 120.234
    }
    '''

Good catch on the influxdb library version, too. I'll change that as well!

@David00
Copy link
Owner

David00 commented Feb 16, 2025

Hey @kizniche, those changes are put in place now on the develop-v0.4.0 branch.

@kizniche
Copy link
Contributor Author

Thanks for the update! I'll be able to test very soon, as I just got another set of hardware (my previous hardware is currently in use in one of my remote systems).

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants