Skip to content

feat: feat add battery registers and read - #1

Open
seixasxbr wants to merge 1 commit into
masterfrom
multiplus2
Open

feat: feat add battery registers and read#1
seixasxbr wants to merge 1 commit into
masterfrom
multiplus2

Conversation

@seixasxbr

Copy link
Copy Markdown
Collaborator

What is this PR for

feat: add battery registers and read

Checklist

  • Assign yourself in the PR
  • Write unit tests (when relevant)
  • Run rubocop and rake tests locally

feat: initial driver implementation

feat: add battery readings

feat: add executables

feat: add battery alarms and move register to a file

feat: add alarm register for battery

feat: improve namming

doc: add units

fix: use base types

chore: format documentation

chore: format documentation

feat: add diagnostics

feat: improve fields

chore: add balance and mode'

chore: improve ve bus naming and remove some fields

feat: add energy counter

fix: remove methods

split pr

fix: use registers

fix: keep only battery registers

move to batteryregisters

feat: add battery test'

remove unecessary static casts

doc
@seixasxbr

Copy link
Copy Markdown
Collaborator Author

/gemini review

@seixasxbr
seixasxbr requested review from ielson and jhonasiv July 6, 2026 19:05
@seixasxbr seixasxbr self-assigned this Jul 6, 2026
Comment thread src/Driver.cpp
BatteryData data;
uint32_t alarms_val = 0;
for (const auto& ar : BatteryData::alarm_register) {
if (readSingleRegister(m_address, true, ar.register_id) == 2) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Can you turn this "2" into an enum or a constant? Its very confusing what that should mean.

Comment thread src/Driver.cpp
Comment on lines +148 to +152
{
auto regs =
readRegisters(m_address, true, BatteryRegisters::CONSUMED_AMPHOURS_2, 2);
data.consumed_amphours_1 = (static_cast<uint32_t>(regs[0]) << 16) | regs[1];
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I dont see why you need the scope here

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Just to keep regs variable local to this scope, since it is used multiple times in this higher level scope

Comment thread src/Driver.cpp
Comment on lines +184 to +185
// TODO:
// data.connection_information = "";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Will it be done?

Comment thread src/Driver.cpp
Comment on lines +28 to +31
{
auto regs = readRegisters(m_address, true, BatteryRegisters::BATTERY_POWER, 2);
data.battery_power = (static_cast<uint32_t>(regs[0]) << 16) | regs[1];
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why the scope here too?

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.

2 participants