Skip to content

Ve-direct driver #440

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

Merged
merged 66 commits into from
Apr 29, 2025
Merged

Ve-direct driver #440

merged 66 commits into from
Apr 29, 2025

Conversation

pkubanek
Copy link
Contributor

@pkubanek pkubanek commented Jun 6, 2017

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
Copy link
Contributor Author

pkubanek commented Jun 6, 2017

Sorry for failed CI, fixed now.

}

void upsdrv_shutdown(void)
{
Copy link
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?


/* list flags and values that you want to receive via -x */
void upsdrv_makevartable(void)
{
Copy link
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)?

@clepple
Copy link
Member

clepple commented Jun 7, 2017

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

break;
*end = '\0';

snprintf(vn, sizeof(vn), "ve-direct.%s", v_name);
Copy link
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
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).


void upsdrv_initups(void)
{
#ifndef TESTING
Copy link
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.

Copy link
Member

@jimklimov jimklimov Apr 26, 2025

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
Member

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

@AppVeyorBot
Copy link

jimklimov added a commit to pkubanek/nut that referenced this pull request Apr 25, 2025
@jimklimov jimklimov added this to the 2.8.4 milestone Apr 26, 2025
@jimklimov
Copy link
Member

jimklimov commented Apr 26, 2025

TODO: Man page and similar docs...

UPDATE: Done

@jimklimov
Copy link
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
0 of 2 checks passed
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