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

OOP #9

Open
big-vl opened this issue Sep 9, 2022 · 4 comments
Open

OOP #9

big-vl opened this issue Sep 9, 2022 · 4 comments

Comments

@big-vl
Copy link

big-vl commented Sep 9, 2022

I saw a method sendPacket in the code, I wanted to use it, but I had to look at the code further, it turns out this method has privacy. Do you think this is a bad method to use? 🤔

@ftylitak
Copy link
Member

Hello @big-vl

it is not wrong to use it as long as you provide a properly initialized CoapPacket instance as "send" and "sendEx" functions do. Also manually set the coap state:

self.state = self.TRANSMISSION_STATE.STATE_IDLE

May I ask why do you need to use the low level function directly?

@big-vl
Copy link
Author

big-vl commented Sep 12, 2022

Hello @big-vl

it is not wrong to use it as long as you provide a properly initialized CoapPacket instance as "send" and "sendEx" functions do. Also manually set the coap state:

self.state = self.TRANSMISSION_STATE.STATE_IDLE

May I ask why do you need to use the low level function directly?

so I wrote that it would be better to make methods more usable, for example _sendPacket I would know that I can use it but with some limitation. So there are no problems with the code, there is also a question about subscription, as I understand it, according to the specification, such an opportunity exists but is not implemented. For now, I'll have to use sockets without a specification.

@big-vl
Copy link
Author

big-vl commented Sep 12, 2022

@ftylitak You have made a very stable and good library, I am delighted.

@ftylitak
Copy link
Member

Thank you for your kind words.

Indeed, the function should be named _sendPacket. Right now I am a bit hesitant to change it to avoid breaking compatibility with applictions that already use it...though I believe there are none to few of them.

It might be done soon.

Fo the subscription, in case you make it work, we are open for contributions :)

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

No branches or pull requests

2 participants