Skip to content

Ve-direct driver - #440

Merged
jimklimov merged 66 commits into
networkupstools:masterfrom
pkubanek:master
Apr 29, 2025
Merged

jimklimov merged 66 commits into
networkupstools:masterfrom
pkubanek:master

Conversation

@pkubanek

@pkubanek pkubanek commented Jun 6, 2017

Copy link
Copy Markdown
Contributor

Working release of Ve-direct driver for Victron Energy UPS/solar panels monitoring.

@pkubanek pkubanek closed this Jun 6, 2017
@pkubanek pkubanek reopened this Jun 6, 2017
@pkubanek

pkubanek commented Jun 6, 2017

Copy link
Copy Markdown
Contributor Author

Sorry for failed CI, fixed now.

Comment thread drivers/ve-direct.c Resolved
Comment thread drivers/ve-direct.c
}

void upsdrv_shutdown(void)
{

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there certainly no session teardown?

Comment thread drivers/ve-direct.c

/* list flags and values that you want to receive via -x */
void upsdrv_makevartable(void)
{

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Aren't there variables to set up or modify like the port, maybe baud speed (e.g. use a default 19200 but reserve ability to probe related units with other hardware abilities)?

Comment thread drivers/ve-direct.c Resolved
@clepple

clepple commented Jun 7, 2017

Copy link
Copy Markdown
Member

I would prefer that these commits be rebased/squashed rather than merged as-is. The individual commits do not stand on their own.

https://github.com/networkupstools/nut/blob/master/docs/developers.txt#L327

Comment thread drivers/ve-direct.c Outdated
break;
*end = '\0';

snprintf(vn, sizeof(vn), "ve-direct.%s", v_name);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

"ve-direct" is not a valid top-level NUT variable name. Please use the standard names defined, and discuss on nut-upsdev if you need anything not covered here: http://networkupstools.org/docs/developer-guide.chunked/apas01.html

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

"Fixed" by moving to experimental.* namespace. Generally we would need a PV-oriented namespace, I suppose (#1407 also stumbles on that).

Comment thread drivers/ve-direct.c

void upsdrv_initups(void)
{
#ifndef TESTING

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The TESTING #define is not handled elsewhere in NUT, but if you include it, I would recommend adding a case to parse a few static text buffers. This will help other developers who might have to maintain the code later down the road.

@jimklimov jimklimov Apr 26, 2025 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There's actually a lot of it in blazer and nutdrv_qx code... Still not really handled in a centralized fashion, but as a precedent - there certainly is some.

@jimklimov

Copy link
Copy Markdown
Member

Resyncing with main NUT code base after v2.8.3 release.

jimklimov and others added 2 commits April 25, 2025 10:58
Suggestion to RTFM is now centralized in NUT code base

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
@AppVeyorBot

Copy link
Copy Markdown

jimklimov added a commit to pkubanek/nut that referenced this pull request Apr 25, 2025
Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
@jimklimov jimklimov added this to the 2.8.4 milestone Apr 26, 2025
Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
…ls#440]

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
@jimklimov

jimklimov commented Apr 26, 2025 •

Copy link
Copy Markdown
Member

TODO: Man page and similar docs...

UPDATE: Done

…ls monitoring [networkupstools#440]

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
…ix a few typos [networkupstools#440]

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
@jimklimov

Copy link
Copy Markdown
Member

Well, there are a few rough edges remaining, but I think the PR became good enough to merge (and then maybe chisel to perfection as part of common NUT code base - e.g. document the parameters which the INSTCMD can get/set).

Thanks @pkubanek !

@jimklimov
jimklimov merged commit 06ca23e into networkupstools:master Apr 29, 2025
Challado pushed a commit to Challado/nut that referenced this pull request Oct 14, 2026
Ve-direct driver

SVN-Git-Bridge: nut-3e341d2d9c-3fc79f1e8124 06ca23e
Bridge-Author: Jim Klimov <jimklimov+nut@gmail.com>
Bridge-Author-Date: 2025-04-29T16:39:24+02:00
Bridge-Committer: GitHub <noreply@github.com>
Bridge-Committer-Date: 2025-04-29T16:39:24+02:00
From: Jim Klimov <jimklimov+nut@gmail.com>

git-svn-id: file:///mnt/export/subversion/nut/trunk@4337 5f2f2009-99cd-4845-9af7-39d0aa6e3748
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

experimental-namespace This PR or issue deals with names not standardized in nut-names.txt and code would need to change needs-work PR discussion concluded that some work is needed from the contributor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants