Bring up to date with MULPIv4.0-I07#77
Conversation
- Added basic estb settings (TLV217) including IpMode control (217.1) and Snmpv1v2CommunityName 217.53.1
- Ip mode type is uchar
Made the following changes to bring the tool up to date with MULPIv4.0-I07: - filled in the gaps and removed duplicates in the symtable identifiers - corrected TLVs 24.36 and 25.36 from an aggregate to a ushort - added TLVs from MULPIv3.1-I2 and MULPIv4.0-I0, except TLVs 103 and 104, which 2 byte length fields. New decoder/encoder functions are needed to support the 2 byte lengths
|
This project is basically unmaintained. So unless someone else shows up with more information, I'm inclined to just merge this as is. @jeffhowe60 Do you have any thoughts on #73 and/or #59? Maybe with your review we could clear out those too. |
@rlaager To be honest, I almost included the changes for #73 in my request. I was hesitant because it only had a small subset of the TLVs from HOST2.1-I17. But to assist clear the backlog, I can merge those changes into my branch. Regarding #59, I am not sure of the submission. I think the parser is trying to convert the sequence <backslash><space> into <space>, which seems like a normal thing to do. I think the change requested instead changes that sequence into <backslash>, removing the <space>. |
|
If you're not sure about #59, we can keep ignoring that. It seems you're knowledgable about #73, so if you can merge that in in whatever fashion is appropriate, I would appreciate that. Please make sure to retain credit for "Dennis Van Ee dennis.vanee@vecima.com" as appropriate--either as the author of the commits if you keep them, or with "Co-authored-by" if you merge things together. I'm fine with (and personally slightly prefer) multiple commits, so don't feel like you have to merge things together. |
…docsis Merged change authored by Dennis Van Ee (dennis.vanee@vecima.com) Added basic estb settings (TLV217) including IpMode control (217.1) and Snmpv1v2CommunityName 217.53.1
|
Changes for #73 have been merged into this request, and will resolve that issue. |
AdrianSimionov
left a comment
AdrianSimionov
left a comment
There was a problem hiding this comment.
I am OK with the changes. What I would extra do, is add test cases for all the newly introduced TLVs. That would protect against breaking backward compatibility.
|
I merged it. Test cases would be nice. If you want to do that, please do so. It'll need to be another PR, but that's no big deal. |
4 participants
Made the following changes to bring the tool up to date with MULPIv4.0-I07: