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

Upstreaming improvements from 42ity/nut fork #1316

Open
30 of 34 tasks
jimklimov opened this issue Feb 25, 2022 · 2 comments
Open
30 of 34 tasks

Upstreaming improvements from 42ity/nut fork #1316

jimklimov opened this issue Feb 25, 2022 · 2 comments
Assignees
Labels
CI Entries related to continuous integration infrastructure (historically also recipes like Makefiles) DMF NUT Data/Dynamic Mapping File/Format/Functionality feature enhancement nut-scanner patch refactor/fightwarn PR or issue proposal to improve code maintainability without functional changes, or to fix warnings SNMP
Milestone

Comments

@jimklimov
Copy link
Member

jimklimov commented Feb 25, 2022

A lot of work on NUT was done under the auspices of Eaton-backed 42ity project, as well as some earlier bundled releases that became part of its codebase. While much of the development was cross-posted for PRs, some was only pursued in the fork to perfect there first. 42ity took the decision to eventually move from the burden of maintaining a separate fork (thereby regularly solving merge conflicts) towards using the "upstream NUT" codebase directly, so I imported the "FTY" branch and identified the largest differing subjects to reconcile between the two:


Notes about known feature changes between the forks (as of comparison in early 2022, before some of these changes got upstreamed in e.g. #320):

  • mibs=eaton_epdu
    ** added "device.N.outlet.switchable" fields
    ** renamed the value of "ambient.contacts.N.status" from "opened" to "open"; note they are "opened" in other MIBs as seen below
-ambient.contacts.1.status: opened
+ambient.contacts.1.status: open
+outlet.switchable: yes

** "device.N.input.feed.desc", "device.N.outlet.M.name", separated "device.N.outlet.group.M.name" vs. "device.N.outlet.group.M.desc" (also added "device.N.outlet.switchable")
** NOTE: compared to those master-branch builds, LOST phase info "outlet.group.N.phase" however maybe only for single-phase devices? seems there is a rename for three-phased ones:

### 1ph:
-outlet.group.1.phase: L1
-outlet.group.2.phase: L1

### 3ph:
+outlet.group.1.desc: Section A
-outlet.group.1.name: Section A
-outlet.group.1.phase: L1
+outlet.group.1.name: A
+outlet.group.1.phase: 1-N
+outlet.group.2.desc: Section B
-outlet.group.2.name: Section B
-outlet.group.2.phase: L2
+outlet.group.2.name: B
+outlet.group.2.phase: 2-N
+outlet.group.3.desc: Section C
-outlet.group.3.name: Section C
-outlet.group.3.phase: L3
+outlet.group.3.name: C
+outlet.group.3.phase: 3-N

...and in at least one case for one phase also (but that may be some experimental ePDU? its data dump includes three separate input phase data collections, but just one outlet group... can that be a three-phase outlet actually?):

+input.feed.desc: Feed A
+outlet.1.name: A1
+outlet.group.1.desc: Section A
-outlet.group.1.name: Section A
-outlet.group.1.phase: L1
+outlet.group.1.name: A
+outlet.group.1.phase: 1-N
  • mibs=pw
    ** between 42ity/nut versions, iteratively added lots of info categories for ambient sensors; listing some unique-name samples here:
+ambient.1.contacts.1.config: normal-opened
+ambient.1.contacts.1.name: EMPDT1H1C2 @1-C1
+ambient.1.contacts.1.status: inactive
...
+ambient.1.humidity.status: good
+ambient.1.id: d01fd439-a44e-5367-bc99-5b5108c9abc5
+ambient.1.mfr: Eaton
+ambient.1.model: Eaton EMPDT1H1C2
+ambient.1.name: EMPDT1H1C2 @1
+ambient.1.present: no
+ambient.1.serial: GBxxxxxxxx
+ambient.1.temperature.status: good
+ambient.1.temperature.unit: celsius
...
+ambient.contacts.1.name: EMPDT1H1C2 @1-C1
+ambient.contacts.2.name: EMPDT1H1C2 @1-C2

...and "outlet.N.status" and "outlet.switchable" where applicable (UPS):

...
+outlet.1.status: on
+outlet.2.status: on
+outlet.switchable: yes

** compared to upstream, added "ambient.contacts.N.status" status (note "opened" not "open" here, while "ambient.N.contacts.M.status" flip between "active"/"inactive") and changed "ups.type" reading, and added "outlet.N.status" and "outlet.switchable" for UPS:

+ambient.contacts.2.name: EMPDT1H1C2 @1-C2
+ambient.contacts.2.status: opened
+outlet.1.status: on
+outlet.2.status: on
+outlet.switchable: yes
-ups.type: high efficiency
+ups.type: On-Line UPS, Single Phase
  • mibs=pxgx_ups
    ** added "ambient.contacts.N.name" and "outlet.N.status"
    ** Compared to master builds (same for 20190711, 20211102_a12302c90ae) added more (note: "opened" not "open"):
+ambient.contacts.1.name: Input \#1
+ambient.contacts.1.status: opened
+ambient.contacts.2.name: Input \#2
+ambient.contacts.2.status: closed
+device.contact: Team
+device.location: Rack
+outlet.1.status: on
+outlet.2.status: on
-ups.type: normal
+ups.type: On-Line UPS, Single Phase
  • mibs=mge
    ** Compared to master builds, added "ambient.contacts.N.status" (note: "opened" not "open") and "device.contact"/"device.location":
+ambient.contacts.1.status: opened
+ambient.contacts.2.status: closed
+device.contact: Team
+device.location: Rack
  • mibs=ietf
    ** Compared to master builds, added device.contact and device.location like above

Also it would help migrations and refactoring with peace of mind, if the 42ity/nut codebase passed same CI requirements as the upstream NUT. Some notes in this regard:

The work about bug-fixing is quite iterative and "simple" structurally, after all the recent improvements that landed in NUT and its recipes already:

:; git clone -b FTY https://github.com/42ity/nut .
:; BUILD_WARNFATAL=yes BUILD_WARNOPT=medium CANBUILD_DRIVERS_DMF=yes BUILD_TYPE=default-alldrv ./ci_build.sh

...and read the warnings which pop up, fix them, make until the recipe looks okay.

Note how many of these warnings follow some same pattern; it makes sense to address each "family" separately, possibly coming up with some common systemic solution.
In a few cases warnings may be mis-fires from the static analyser in compiler, and pragmas would be used to quiesce them - see precedent to check and define the platform support for particular pragmas and warning options.
In many cases note the mess with integer types - prefer fixed-width types (rather than architecture-dependent int/short/long) and particular signedness following system headers; particularly note e.g. ssize_t (rarely) or size_t (often) for read sizes, array sizes, memory buffer sizes etc. instead of opportunistic int. All this is related to range-checks in many cases, with "tautology" quiesced by pragmas because on some platforms the check is pointless and protects against the other platforms.
Then post a PR to have NUT CI farm go at it, using a hundred combos on different platforms and toolkit versions.
See docs/config-prereq.txt for different platform setup notes, to reproduce faults in local VMs.
Repeat until NUT CI passes green.

Currently known issues fall into just a few buckets:
Helper script _wsum to summarize issues:

#!/bin/sh
date; \
echo "Making whatever succeeds"; \
make -j -k >/dev/null 2>&1; \
echo "Making for error report"; \
make -k > wall.tmp 2>&1 ; \
mv -f wall.tmp wall; \
egrep '\[K?-Werror' < wall > wsum \
&& awk '{print $NF}' < wsum | sort | uniq -c | sort -n > wsum.count \
&& wc -l wsum >> wsum.count \
&& cat wsum.count

Example run:

$ ./_wsum 
February  2, 2022 at 02:58:20 PM CET
Making whatever succeeds
Making for error report
      1 [-Werror=pedantic]
      1 [-Werror=unused-parameter]
     39 [-Werror]
    697 [-Werror=missing-field-initializers]
738 wsum

The few errors above are about definitions:

scan_snmp.c:106:13: error: redefinition of typedef 'bool_t' [-Werror=pedantic]

../../include/config.h:29: error: "BINDIR" redefined [-Werror]
../../include/config.h:32: error: "CGIPATH" redefined [-Werror]
../../include/config.h:35: error: "CONFPATH" redefined [-Werror]
../../include/config.h:47: error: "DATADIR" redefined [-Werror]
../../include/config.h:51: error: "DEFAULT_DMFNUTSCAN_DIR" redefined [-Werror]
../../include/config.h:55: error: "DEFAULT_DMFNUTSCAN_RES_DIR" redefined [-Werror]
../../include/config.h:58: error: "DEFAULT_DMFSNMP_DIR" redefined [-Werror]
../../include/config.h:61: error: "DEFAULT_DMFSNMP_RES_DIR" redefined [-Werror]
../../include/config.h:64: error: "DRVPATH" redefined [-Werror]
../../include/config.h:812: error: "HTMLPATH" redefined [-Werror]
../../include/config.h:815: error: "LIBDIR" redefined [-Werror]
../../include/config.h:818: error: "LIBEXECDIR" redefined [-Werror]
../../include/config.h:914: error: "SBINDIR" redefined [-Werror]

drivers/eaton-pdu-marlin-mib.c:825:42: error: unused parameter 'daisy_dev_list' [-Werror=unused-parameter]

...and a large amount of same error with missing field initializers for snmp_info_t tables (DMF codebase has more fields there, aiming for more conversion function types; if that approach survives - an initialization macro may be due to substitute the NULLs where no value was given).

@jimklimov jimklimov added enhancement nut-scanner SNMP CI Entries related to continuous integration infrastructure (historically also recipes like Makefiles) refactor/fightwarn PR or issue proposal to improve code maintainability without functional changes, or to fix warnings labels Feb 25, 2022
@jimklimov jimklimov added this to the 2.8.1 milestone Apr 24, 2022
@jimklimov jimklimov modified the milestones: 2.8.1, 2.8.2 Jan 6, 2023
@jimklimov jimklimov added the DMF NUT Data/Dynamic Mapping File/Format/Functionality feature label Oct 16, 2023
@jimklimov jimklimov added the patch label Dec 4, 2023
@jimklimov
Copy link
Member Author

Updated checklist above with achievements around PR #2275 which merged current master into FTY branch so it built green for the first time, and ported back the installer, nutconf and some smaller fixes to minimize the difference - those became part of recent NUT v2.8.2 release.

@jimklimov
Copy link
Member Author

jimklimov commented Jul 4, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Entries related to continuous integration infrastructure (historically also recipes like Makefiles) DMF NUT Data/Dynamic Mapping File/Format/Functionality feature enhancement nut-scanner patch refactor/fightwarn PR or issue proposal to improve code maintainability without functional changes, or to fix warnings SNMP
Projects
None yet
Development

No branches or pull requests

2 participants