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

Fix prompt pattern for model/comware.rb to handle ASCII Bell response +fix minor README.md typo #1094

Merged
merged 3 commits into from
Nov 11, 2017

Conversation

udha
Copy link
Contributor

@udha udha commented Nov 10, 2017

Changes prompt /^\0*(<[\w.-]+>)$/ to prompt /^\0*(<[\w.-]+>).?$/ to handle situations where ASCII Bell character is sent at the end of the prompt by Comware HP/H3C/3Com devices.

Also fixes minor type of 'HTT' to 'HTTP' in README.md file.

@@ -42,6 +42,10 @@ class Comware < Oxidized::Model
end

post_login 'screen-length disable'
# optionally you can add into system-view:
Copy link
Contributor

Choose a reason for hiding this comment

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

The screen-length disable command is run all the time anyway so I'm not sure those notes are useful.

@laf
Copy link
Contributor

laf commented Nov 10, 2017

@Ultra2D / @amarti2038 - As you were the last two users to edit this model, would either of you be able to test this or confirm it's ok to merge?

@udha
Copy link
Contributor Author

udha commented Nov 10, 2017

Do you want me to remove the line disable comments?

@udha
Copy link
Contributor Author

udha commented Nov 10, 2017

I can also provide a debug dump of a connection failing before this change and then working successfully after the change if that helps

@Ultra2D
Copy link
Contributor

Ultra2D commented Nov 10, 2017

Works for me, but note that I only manage 1 Comware stack (2 devices).

@laf
Copy link
Contributor

laf commented Nov 10, 2017

Do you want me to remove the line disable comments?

Yes please :)

I can also provide a debug dump of a connection failing before this change and then working successfully after the change if that helps

Probably not necessary, just trying to avoid breaking anything :)

@udha
Copy link
Contributor Author

udha commented Nov 11, 2017

Yes please :)

All cleared, thanks for the guidance 👍

@laf laf merged commit 4203e00 into ytti:master Nov 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants