Skip to content
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

Ve-direct driver #440

Open
wants to merge 56 commits into
base: master
Choose a base branch
from
Open

Ve-direct driver #440

wants to merge 56 commits into from

Conversation

pkubanek
Copy link

@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
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)?


void upsdrv_help(void)
{
printf("Read The Fine Manual ('man 8 ve-direct')\n");
Copy link
Member

Choose a reason for hiding this comment

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

Related to question on configurable -x flags - maybe a bit more is to be said here. Maybe at least the standard wall of text with -a, -D and other standard flags.

@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


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.

@jimklimov jimklimov added the needs-work PR discussion concluded that some work is needed from the contributor label May 24, 2021
@jimklimov jimklimov added the experimental-namespace This PR or issue deals with names not standardized in nut-names.txt and code would need to change label Nov 14, 2021
…ed during fightwarn (so might add regressions) without regard to versioning
…and consult drivers/Makefile.am for list names to require certain dependencies
…sx) drivers - do not default to require them everywhere
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.

3 participants