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.

Sign up for GitHub

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

Jump to bottom

Fix issues in network adapter and core getStatus(), getStatuses() #90

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Fix issues in network adapter and core getStatus(), getStatuses() #90

wants to merge 2 commits into from

Conversation

Xelio
Copy link

@Xelio Xelio commented Apr 7, 2024

  • core getStatus(): change buffer.write() to adapter.write(), buffer.write() does not send data to print immediately.
  • core getStatuses(): change to retrieve status one by one, because network printer does not return all 4 status in the same read() operation.
  • core: add error handling in getStatus() and getStatuses().
  • network adapter open(): remove err from connectListener of .connect(), it does not have err param.
  • network adapter read(): change .on() to .once(), callback should only run once for current read operation.
  • network adapter: add read timeout. When read timeout, pass empty buffer to callback function.
  • network adapter: add test of getStatus(), getStatuses().
  • export StatusJSON, StatusJSONElement, StatusJSONElementSingle, StatusJSONElementMultiple

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Apr 7, 2024 •
edited
Loading

🦋 Changeset detected

Latest commit: 98606bd

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@node-escpos/core Minor
@node-escpos/network-adapter Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Apr 7, 2024
@Xelio
Copy link
Author

Xelio commented Apr 7, 2024

Hi @dohooo,
This PR is mainly for network printer. I only has network printer, so can't test it on USB/Serial/BT printer.
Would you help to test on USB/Serial/BT printer, and see if there is any problem?

@dohooo
Copy link
Contributor

dohooo commented Apr 8, 2024 •
edited
Loading

⚠️ No Changeset found

Latest commit: 4a2bdbc

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets
When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@Xelio Thanks for your effort, awesome work! But before I merge it, could you add a changeset for these changes? BTW, Can you add unit test for this PR.

@dohooo dohooo changed the base branch from main to develop April 8, 2024 04:27
@KylianJay
Copy link

Looking forward to this release! Will probably fix many of the issues I am having, as I use Network printer for my main use-case. No rush, but add a changeset and unit test please?

@watcharakrit
Copy link

Hoping to see this merged soon!

Xelio Cheong added 2 commits July 4, 2024 23:58
Improvement for core and network adapter.
1f430c8 
core:
- core getStatus(): change buffer.write() to adapter.write(), becuase buffer.write() does not send data to print immediately.
- core getStatuses(): change to retrieve status one by one, because network printer does not always return all 4 status in a single read() operation.
- Export class StatusJSON, StatusJSONElement, StatusJSONElementSingle, StatusJSONElementMultiple
- Add test for core getStatus() and getStatuses()

network adapter:
- network adapter open(): remove err from connectListener of .connect(), it's defination does not have err param.
- network adapter read(): change .on() to .once(), using .on() will run previous callback again in next read() operation, which is not the expected behavior.
- Add read timeout for network adapter. When timeout occur, it will pass empty buffer to callback function.
- Add error handling in getStatus() and getStatuses().
- Add test of getStatus(), getStatuses() for network adapter.
Add changeset
98606bd
@Xelio Xelio force-pushed the fix-network-adapter branch from 68ff7b0 to 98606bd Compare July 4, 2024 16:10
@Xelio
Copy link
Author

Xelio commented Jul 4, 2024

Hi @dohooo , Tests and changeset added, please take a look, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Xelio @dohooo @KylianJay @watcharakrit