Skip to content

Add test for Request.#3

Open
Sam-Serpoosh wants to merge 2 commits intotenderlove:masterfrom
Sam-Serpoosh:add-tests-for-request
Open

Add test for Request.#3
Sam-Serpoosh wants to merge 2 commits intotenderlove:masterfrom
Sam-Serpoosh:add-tests-for-request

Conversation

@Sam-Serpoosh
Copy link

  • Test for dlen, packet_body, checksum, etc.
  • Delete indirect tests for request.
  • Couple of minor Refactoring on Request checksum, etc.

- Test for dlen, packet_body, checksum, etc.
- Delete indirect tests for request.
- Couple of minor Refactoring on Request checksum, etc.
Copy link
Owner

Choose a reason for hiding this comment

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

Please don't change this, I'm packing with multiple 'C' because there is a certain number of characters. You could change the format string to specify a number rather than repeating 'C' over and over, but 'C*' is different.

Copy link
Author

Choose a reason for hiding this comment

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

I actually had some doubts about making this change to be honest and I wanted to see your feedback about it! I thought to myself that since there's EXACTLY 6 elements in the header array then it should be clear in the packing format with 'CCCCCC'! Thanks for the feedback I'll change that back!

@Sam-Serpoosh
Copy link
Author

Also I'm reading/working on Sphero at the moment a little bit and I'm planning to add some more functionalities to this gem for controlling Sphero ball! Thanks again for all of your feedbacks! I'll make the changes and looking forward to more feedbacks from you!

Regards,

- Address @tenderlove feedbacks on the previous pull request.
- Put back %256 in checksum calculation
- change require_relative to require.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants