Add Client class

[chipzz]

Refactor TrapListener and Agent into a Listener class, which deri- ves from the Receiver class. This Receiver class also serves as the base for a new Client class that provides an SNMP client API.

[chipzz]

This is mostly finished but still WIP in part. Everything except the actual client API should be good though.

As far as the client API goes, the parts I was able to test (GetRequest/ GetNextRequest) work, but I'm currently hard-coding the SNMP version (which works for my specific needs, but may not be the best solution). I could easily add a version parameter, but I'm not sure what the best way to do that would be, without exploding the number of arguments required for each function. I could use an option object as argument, but I would like to hear your thoughts on the matter.

[chipzz]

Fixed a small bug I introduced in the checking of the callback in the listener.

[chipzz]

Remove cl.js(on) and replace by 3 proper examples (snmpget.js, snmpset.js, snmpwalk.js)

[chipzz]

Added version parameter

[chipzz]

Here's a breakdown of all the changes:

Refactoring of existing code: agent.js and trap_listener.js lib/receiver.js: base interface for receiving SNMP packets. _recv, _process_msg, _augment_msg and createSocket (part of bind) move here. Moving _recv here is the only incompatible change in this whole branch: the version from agent.js has 3 parameters, so classes deriving from TrapListener will have to accept a third argument (which they can simply ignore); lib/listener.js: bind and close move here from lib/trap_listener.js: move _recv, bind and close to receiver.js and listener.js; lib/agent.js: move _recv, bind and close to receiver.js and listener.js

Bugfixes: lib/protocol/pdu.js: make non_repeaters and max_repetitions actually work

New API: lib/index.js: wrapper method; lib/client.js: the implementation of the client API itself.

Examples: snmpbulkget.js snmpget.js snmpset.js snmptrap.js snmpwalk.js

Build and jsstyle/jsl issues: Makefile tl.js lib/protocol/uint64_t.js

[chipzz]

Additional note: snmpinform.js is a symlink to snmptrap.js; invoking it as snmpinform.js will pass a callback, invoking it as snmptrap.js won't.

[davepacheco]

A few comments:

• "make prepush" emits a warning: /home/dap/node-snmpjs/lib/client.js(107): warning: identifer data hides an identifier in a parent scope
• client.js: looking up the system's hostname in DNS is not valid. The hostname could refer to some other host, or it could not be in DNS at all. Can we eliminate this?
• We do we use _unref()? Is this so clients don't have to call close()? Why not just make them call close() so that it's explicit?
• What warnings are the jsl ignore blocks hiding in uint64_t.js?

Thanks.

[chipzz]

1) Will fix

2) SNMP provides the functionality to send traps on someone else's behalf. That's what this field is used for. I seriously doubt that a lot of software even considers this field when looking at traps it receives (I suspect they will look at the source ip instead, which can be different). Usually you're not sending traps on anyone else's behalf except your own. The way it works is as follows: a) the user can specify an agent_addr in options themselves if they so desire; b) if they don't, the _hostip fallback gets used. This fallback works as follows:

• initialized to "127.0.0.1";
• subsequently gets replaced by hostname() if and only if the look-up succeeds. I choose the hostname look-up approach as that is what the net-snmp implementation also does ( http://www.net-snmp.org/docs/man/snmptrap.html ), so that seems like a reasonable solution, especially given it is fallback behavior in the first place (the user can specify options.agent_addr if they so desire).

3) The unref is analogous to: https://nodejs.org/api/dgram.html#dgram_socket_unref and https://nodejs.org/api/net.html#net_socket_unref and https://nodejs.org/api/net.html#net_server_unref Which allows CLI programs to exit after obtaining the desired results instead of looping indefinitely. But I guess you're right, I could merge this into close. Will do.

4) The specifc output of jsl for this file is: /snmpjs/lib/protocol/uint64_t.js(57): warning: redeclaration of s /snmpjs/lib/protocol/uint64_t.js(61): warning: redeclaration of d

Which are only previously declared in the other else branch (lines 42 and 49 respectively) and are false positives. I don't know enough about this code and it looks too risky to fix this properly in this branch (it's unrelated to the rest of the diff after all).

[chipzz]

Fixed the above issues: 1) Renamed data parameter to value; 3) Renamed unref to close; 4) Decided to fix the warnings properly after all, wasn't that difficult.

That being said, a number of thoughts on the current state of the client API.

• It has all function names starting with a capital, which is an artifact of a search and replace on the constants. This is not in line with the rest of the API and I'm open to change it.
• We only provide the SNMP primitives. snmpwalk is included in an example, but we could put it either in lib/index.js as a convenience function or in lib/client.js (part of the Client class, or simply as a separate function). Not sure what to do here. Your feedback?

[davepacheco]

Thanks for making those changes! On your follow-up questions: I would make the API match up, and I don't have much opinion on snmpwalk, but if we're on the fence, I'd slightly prefer to avoid adding a new interface.

[chipzz]

Made the requested changes to the client API, and left snmpwalk as it was. Ready to land?

[chipzz]

WRT your question on IRC, should we bump the major? Probably not IMO, the only incompatible change, as mentioned above, is in the _recv function, but given that it starts with an underscore I would say it's a private function so we're not really breaking API. I would however suggest bumping the minor to 0.2.0, given that it's quite a big addition. Shall I do that or will you do that in a follow-up diff?

[davepacheco]

Merged! Thanks again.

Do you want a new version (0.2.0) published to npm?