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

feat: Allow close method to be used with Promises or callbacks #562

Merged
merged 2 commits into from
Aug 18, 2024

Conversation

grazianogrespan
Copy link
Contributor

First of all a big thank you for this fantastic module!
I was exploring the example folder looking for the close() method and I observed that it might be used in a way that suggests synchronous behavior. polling_TCP.js

Checking the implementation of the close() method I found out that it uses the Node.JS net.Socket class and in particular the socket.end() method which does not guarantee that the socket is fully closed immediately after calling socket.end().

This pull request introduces Promise support to the close method and should not affect the existing usage.
A callback can still be passed as argument.

@yaacov
Copy link
Owner

yaacov commented Aug 18, 2024

@grazianogrespan hi, thank you for the pull request 💚

can you move the code to the "promise" file ? see https://github.com/yaacov/node-modbus-serial/blob/master/apis/promise.js#L63

@grazianogrespan
Copy link
Contributor Author

Thanks for your feedback @yaacov !
On the promise.js file I tried to use the _convert() method but I had to adjust it to make it work with different argument structures.
Let me know if this is fine for you 🙏

@yaacov yaacov merged commit 6649806 into yaacov:master Aug 18, 2024
3 checks passed
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.

2 participants