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

Add EthernetServer() {} constructor #191

Open
sz-coder opened this issue Apr 20, 2022 · 12 comments · May be fixed by #39
Open

Add EthernetServer() {} constructor #191

sz-coder opened this issue Apr 20, 2022 · 12 comments · May be fixed by #39
Labels
topic: code Related to content of the project itself type: enhancement Proposed improvement

Comments

@sz-coder
Copy link

Looking at the source code, only EthernetServer(uint16_t port) : _port(port) { } is supported.

I don't see why it's needed to be specific about the port when constructing an object from EthernetServer.

This behaviour, however, makes it impossible to pool and re-use EthernetServer objects since the port must be known when constructing the object.

@per1234 per1234 added topic: code Related to content of the project itself type: enhancement Proposed improvement labels Apr 20, 2022
@JAndrassy
Copy link
Contributor

duplicate of #39

@per1234
Copy link
Contributor

per1234 commented Apr 20, 2022

Thanks for pointing out #39 @JAndrassy. Since that is a PR, we can keep this issue open. I have linked the PR to this issue so that if it is merged the issue will automatically be closed.

Even though we don't have a policy that every PR must address an issue (meaning the PR author must create an issue before the PR if there is not an existing one) as some other projects do, I do see some merit in that approach. That merit is outweighed by the burden such a policy places on the contributor IMO, but in cases where the issue is created independently there is no harm in having an issue to track the defect or enhancement request, with the PR serving as one proposal for resolving it.

@JAndrassy
Copy link
Contributor

Per, while the PR is not closed, comments suggest it will not be merged and there is a workaround too.

@sz-coder
Copy link
Author

Per, while the PR is not closed, comments suggest it will not be merged and there is a workaround too.

Why will this PR not be merged? It doesn't look like a breaking API change.

@sz-coder
Copy link
Author

and there is a workaround too.

And what is that workaround?

@JAndrassy
Copy link
Contributor

and there is a workaround too.

And what is that workaround?

server = EthernetServer(port); // change the port

@sz-coder
Copy link
Author

and there is a workaround too.

And what is that workaround?

server = EthernetServer(port); // change the port

Not a feasible workaround for my purposes; I can't call the constructor twice.

@JAndrassy
Copy link
Contributor

and there is a workaround too.

And what is that workaround?

server = EthernetServer(port); // change the port

Not a feasible workaround for my purposes; I can't call the constructor twice.

why not? it doesn't do anything only sets the port

@sz-coder
Copy link
Author

sz-coder commented Apr 21, 2022

Because I'm doing something similar to this:

    // pool of EthernetUDP instances
    static EthernetUDP _udp_pool[5];

    // ...

    EthernetUDP *getNextFromPool(int port) {
        int free_element = _getNextFreeIndex();

        if (0 > free_element) return NULL;

        EthernetUDP *i = &_udp_pool[free_element];

        // call i->end(); if was in use before

        i->begin(port);

        return i;
    }

This works fine for EthernetUDP since it has a begin() method we can pass the port to.

However this approach does not work with EthernetServer.

How exactly am I going to call a constructor for a global object again?

@sz-coder
Copy link
Author

I want to make clear I want to reuse and not re-create instances of EthernetServer.

@JAndrassy
Copy link
Contributor

JAndrassy commented Apr 21, 2022

I want to make clear I want to reuse and not re-create instances of EthernetServer.

how reuse? you have to call end() and begin() if you change the port.
the server = EthernetServer(port); creates an object and copies its contents (port) into the global object.
btw there is no end() so the chip will still listen on the port used with the previous begin()

@sz-coder
Copy link
Author

sz-coder commented Apr 27, 2022

how reuse?

Re-use in the sense that I don't have to create a new object instance to start listening and/or change the listening port.

I already gave an example with EthernetUDP: with this class, it's possible to just call end() and then begin() to start listening on a new port.

I'm aware that the instance itself isn't holding any important information and acts like an access wrapper to the W5x00 Chip. I get that.

C++ is a complex language and I don't wanna play around with things that are "weird" - if you know what I mean - just because something "works" doesn't mean it's correct. This is especially true for languages like C and C++.

I sincerely hope you'll implement this, otherwise I see myself forced to use my own implementation which would be a shame tbh.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants