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

snmp mib2nut wrapper macros #2285

Merged

Conversation

jimklimov
Copy link
Member

@jimklimov jimklimov commented Jan 27, 2024

Idea raised in PR #2275 allowing to juggle several codebases (main and DMF/FTY until merged) with different C structure property counts and minimal difference in comparable sources. For this, macros snmp_info_default and info_lkp_default (and a few for function pointers within) are provided.

This PR also clearly ends the tables with an entry literally named ..._sentinel so it is role is well understood (and spelling of the entry fields is never botched), and it is better visible where someone forgot to add one. So a bonus for maintainability.

NOTE: Not updating sub-driver versions (except for a few files with mapping content changes) since the tables after C pre-processor expanding the macros should be equivalent to codebase before this PR.

…ULL fields for snmp_info_t and info_lkp_t [DMF]

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
… propose snmp_info_default(...) instead of fixed C structure items

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
… propose info_lkp_default(...) instead of fixed C structure items

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
…e short-hand snmp_info_sentinel and info_lkp_sentinel

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
…s2l() and info_lkp_fun_s2l() wrappers

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

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
@jimklimov jimklimov added SNMP refactor/fightwarn PR or issue proposal to improve code maintainability without functional changes, or to fix warnings DMF NUT Data/Dynamic Mapping File/Format/Functionality feature labels Jan 27, 2024
@jimklimov jimklimov added this to the 2.8.2 milestone Jan 27, 2024
@jimklimov jimklimov requested a review from aquette January 27, 2024 21:57
…acros

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
/* no-op without DMF extensions */
# define info_lkp_fun_s2l(_1, _2, _3) {_1, _2, NULL, NULL}
# define info_lkp_nuf_vp2s(_1, _2, _3) {_1, _2, NULL, NULL}
# endif /* WITH_DMFMIB */
Copy link
Member Author

Choose a reason for hiding this comment

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

There is a mapping precedent in original eaton-marlin MIB, which uses the third pointer (so info_lkp_fun_s2l) in DMF branch, but the second pointer as the only one available in master. Not sure OTOH which one is correct; it could have been the case why the extra couple appeared in the first place.

Ideally, someone in the context should revise it more attentively, maybe test build of this branch against a device. @aquette - would you be up to this?

At the moment info_lkp_fun_s2l for non-DMF wrapper would ignore that function. Maybe they should re-use the first and second pointers, if the old (current master) codebase was so inclined?

Copy link
Member Author

@jimklimov jimklimov Jan 29, 2024

Choose a reason for hiding this comment

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

NOTE: Updated the macros to conflate the two s2l (string-to-long) and vp2s (void-pointer-to-string) macro names and so mapping table fields of the vanilla NUT, which became role-separated (forward and reverse fun/nuf conversions) in the DMF branch.

Comment on lines 284 to 288
static info_lkp_t marlin_device_count_info[] = {
{ 1, "dummy", NULL, marlin_device_count_fun },
{ 0, NULL, NULL, NULL }
/* TOTHINK: DMF branch used info_lkp_fun_s2l()? */
info_lkp_nuf_s2l(1, "dummy", marlin_device_count_fun),
info_lkp_sentinel
};
Copy link
Member Author

Choose a reason for hiding this comment

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

Here is the part about function wrappers (second or third pointer) I mentioned in another comment. The DMF/FTY branch has it different than this code, e.g.

  • #if WITH_SNMP_LKP_FUN
    /* Note: marlin_device_count_fun() is defined in eaton-pdu-marlin-helpers.c
    * Future work for DMF might provide a same-named routine via LUA-C gateway.
    */
    #if WITH_SNMP_LKP_FUN_DUMMY
    long marlin_device_count_fun(const char *daisy_dev_list)
    { return 1; }
    #endif // WITH_SNMP_LKP_FUN_DUMMY
    static info_lkp_t marlin_device_count_info[] = {
    { 1, "dummy"
    #if WITH_SNMP_LKP_FUN
    , NULL, NULL, marlin_device_count_fun, NULL
    #endif
    },
    { 0, NULL
    #if WITH_SNMP_LKP_FUN
    , NULL, NULL, NULL, NULL
    #endif
    }
    };
    #else // if not WITH_SNMP_LKP_FUN:
    /* FIXME: For now, DMF codebase falls back to old implementation with static
    * lookup/mapping tables for this, which can easily go into the DMF XML file.
    */
    #endif // WITH_SNMP_LKP_FUN

Copy link
Member Author

Choose a reason for hiding this comment

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

Note: the PR was later amended to at least use the pointer to function in non-DMF builds (conflate with the other ..._s2l() method, though).

…ib2nut-wrappers

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

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
…to snmp_info_default() macros

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
…_info[] => eaton_ats16_nm2_ambient_drycontacts_info[] to match other table naming patterns

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

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
…ith info_lkp_fun_s2l() and info_lkp_nuf_vp2s()

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

This comment was marked as outdated.

…kp_sentinel

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
drivers/delta_ups-mib.c Outdated Show resolved Hide resolved
Comment on lines +125 to +127
info_lkp_default(11, "BYPASS"), /* maintenanceBypass */
info_lkp_default(11, "OL"), /* essMode */
/* FIXME: is "11" correct here or in the line above? */
Copy link
Member Author

Choose a reason for hiding this comment

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

@aquette : the two 11 values here, also in original code before the change - which one is right and which is a typo?

Copy link
Member Author

Choose a reason for hiding this comment

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

Offloaded to #2303

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
drivers/raritan-px2-mib.c Outdated Show resolved Hide resolved
@jimklimov
Copy link
Member Author

@arnaudquette-eaton : I have finally gone over the differences visually, most things fit well; I would however welcome a review for the few content-related questions raised above.

Re-apply commit d3b6ea4 to modernized codebase of PR networkupstools#2285
and newly cover drivers/hpe-pdu-mib.c as well

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
@jimklimov jimklimov force-pushed the issue-2275-snmp-mib2nut-wrappers branch from 270ea61 to 91f107b Compare February 9, 2024 13:37
@jimklimov
Copy link
Member Author

CI fault is not relevant here (busy port issue during self-tests).

@jimklimov
Copy link
Member Author

Offloaded remaining questions to separate issues so they can be addressed later if needed: they are not strictly related to changes of this PR, just were found during the big look at lots of code for its review.

Closes: networkupstools#2302

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
…nticipated by this vendor MIB

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
…2_outlet_status_info[] sentinel)

Closes: networkupstools#2304

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
@jimklimov jimklimov force-pushed the issue-2275-snmp-mib2nut-wrappers branch from 87e3394 to 5621018 Compare February 10, 2024 22:49
@jimklimov jimklimov merged commit d950fa7 into networkupstools:master Feb 11, 2024
12 checks passed
@jimklimov jimklimov deleted the issue-2275-snmp-mib2nut-wrappers branch February 11, 2024 10:38
jimklimov added a commit that referenced this pull request Dec 1, 2024
…le [#2285]

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DMF NUT Data/Dynamic Mapping File/Format/Functionality feature refactor/fightwarn PR or issue proposal to improve code maintainability without functional changes, or to fix warnings SNMP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant