Skip to content

feat(em511): Add the driver with some functions - #27

Merged
Dexter9532 merged 1 commit into
mainfrom
driver_em511
Oct 28, 2025
Merged

feat(em511): Add the driver with some functions#27
Dexter9532 merged 1 commit into
mainfrom
driver_em511

Conversation

@Dexter9532

Copy link
Copy Markdown
Contributor

also added some tests, should run!

@Dexter9532
Dexter9532 force-pushed the driver_em511 branch 19 times, most recently from 979ace3 to 0dc84a7 Compare October 24, 2025 12:19
@Dexter9532
Dexter9532 requested a review from mirzak October 24, 2025 12:32
@Dexter9532
Dexter9532 force-pushed the driver_em511 branch 2 times, most recently from ca44418 to c2b0dca Compare October 24, 2025 13:03

@mirzak mirzak left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good start. We still have some work to do :).

Comment thread src/em511/em511.py Outdated
Comment thread src/em511/em511.py Outdated
Comment thread src/em511/em511.py Outdated
Comment thread src/em511/em511.py Outdated
Comment thread src/em511/em511.py Outdated
Comment thread src/em511/em511.py Outdated
Comment thread src/em511/em511.py Outdated
Comment thread src/em511/em511.py
Comment thread src/em511/em511.py
Comment thread src/em511/em511.py Outdated
@MiladMakdesi
MiladMakdesi self-requested a review October 24, 2025 16:27
@mirzak mirzak mentioned this pull request Oct 27, 2025
@Dexter9532
Dexter9532 force-pushed the driver_em511 branch 2 times, most recently from 50554e4 to 663511a Compare October 27, 2025 12:32
@Dexter9532
Dexter9532 requested a review from mirzak October 27, 2025 12:32
Comment thread src/em511/em511.py Outdated
Comment thread src/em511/em511.py
Comment thread src/em511/em511.py Outdated
Comment thread src/em511/em511.py Outdated
Comment thread src/em511/em511.py Outdated
Comment thread src/em511/em511.py Outdated
Comment thread src/em511/test_em511.py
@Dexter9532
Dexter9532 requested a review from mirzak October 27, 2025 15:37
@Dexter9532
Dexter9532 force-pushed the driver_em511 branch 2 times, most recently from 6770d3b to 484f2f4 Compare October 27, 2025 15:56
Comment thread src/em511/em511.py Outdated
Comment thread src/em511/em511.py Outdated
Comment thread src/em511/em511.py Outdated
Comment thread src/em511/em511.py Outdated
Comment thread src/em511/em511.py Outdated
Comment thread src/em511/em511.py Outdated
Comment thread src/em511/em511.py Outdated
Comment thread src/em511/em511.py Outdated
Comment thread src/em511/em511.py Outdated
Comment thread src/em511/em511.py Outdated
@Dexter9532

Copy link
Copy Markdown
Contributor Author

now everything should be as requested

@mirzak
mirzak marked this pull request as ready for review October 28, 2025 08:44
Copilot AI review requested due to automatic review settings October 28, 2025 08:44

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a new driver for the Carlo Gavazzi EM511 energy meter with Modbus communication support. The implementation includes property-based access to voltage and current readings, as well as password management functionality.

  • Implements the Em511 driver class with read/write operations for Modbus registers
  • Adds comprehensive test coverage for voltage, current, and password operations
  • Replaces placeholder hello function with actual driver implementation

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
src/em511/em511.py Core driver implementation with Modbus register operations for EM511 energy meter
src/em511/test_em511.py Unit tests covering voltage, current, and password read/write operations with edge cases
src/em511/init.py Package initialization exposing the Em511 driver class
src/em511/test_hello.py Removed placeholder test file

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/em511/test_em511.py Outdated
Comment thread src/em511/test_em511.py Outdated
Comment thread src/em511/em511.py
@mirzak

mirzak commented Oct 28, 2025

Copy link
Copy Markdown
Contributor

Great! I marked this as ready for review, which triggers a Copilot review. It found some spelling to fix.

Before approving and merging we need to sort out the packaging. Right now if you run the following:

uv build
uv pip install dist/python_em511-0.0.1-py3-none-any.whl
uv run --python 3.12 python

To import the module I have to use :

import src.em511

This means that is right now shipping the src directory in the package. We do not want that. We most likely need to tweak https://github.com/id8-engineering/python-em511/blob/main/pyproject.toml.

The desired end result is to be able to import using:

import em511

@Dexter9532

Copy link
Copy Markdown
Contributor Author

PR #39 will fix this problem!

@mirzak mirzak left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you rebase this on main and please update the commit message. Something along the lines of:

feat(em511): Add base driver

Signed-off-by: Bobo Bäck Engström <bobo@id8-engineering.io>
@Dexter9532

Copy link
Copy Markdown
Contributor Author

what do mean rebase this on main?

@Dexter9532

Copy link
Copy Markdown
Contributor Author

commit is fixed!

@Dexter9532
Dexter9532 requested a review from mirzak October 28, 2025 09:59
@mirzak

mirzak commented Oct 28, 2025

Copy link
Copy Markdown
Contributor

what do mean rebase this on main?

You already did it :). I mean to run git rebase origin/main to make sure the latest changes from main branch are merged to driver_em511 branch.

@Dexter9532

Copy link
Copy Markdown
Contributor Author

ah yes i understand

@mirzak mirzak left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@Dexter9532
Dexter9532 merged commit 8baa99d into main Oct 28, 2025
20 checks passed
@Dexter9532
Dexter9532 deleted the driver_em511 branch October 28, 2025 10:08
@Dexter9532

Copy link
Copy Markdown
Contributor Author

vammmooos

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