-
Notifications
You must be signed in to change notification settings - Fork 16
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
Got rid of the magic numbers and replaced them with a command map #11
Conversation
Excellent, I've wanted to get rid of those for a long time now :) Why is npm-debug.log in the commit? I guess it can be removed. |
if(!(typeof val === 'number')){ | ||
throw new Error("Value passed to ._write4Bits must be a number"); | ||
} else if (val > 16){ | ||
throw new Error("Value passed to .write4Bits must be 16 or less") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently it's valid to call _write4Bits with a val > 16 and this happens a lot. Because it happens a lot, the error on line 260 will be thrown a lot. For example, the command for SCROLL_LEFT is 0x18, so when scrollDisplayLeft is called, _write4Bits ends up being called twice from _send. In the first call val is 0x01 and in the second call it's 0x18. The error will be thrown in the second call. The same will happen when most characters are printed as most characters have a character code > 16
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the error and the associated test.
It LGTM now. Thank you very much. I'm going to push the green button. |
YAY! |
Got rid of the magic numbers and replaced them with a command map
No description provided.