-
Notifications
You must be signed in to change notification settings - Fork 725
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
Why specify mux when creating GsmClientSim7000 for a TinyGsmSim7000 #623
Comments
This was referenced Dec 14, 2021
I don't remember any reasons not to use a factory - it was just never coded that way and most users seem to stick to only one client so it hasn't come up much. |
Bascy
added a commit
to Bascy/TinyGSM
that referenced
this issue
Jan 5, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
When creating a new
GsmClientSim7000
the constructor asks to specify a mux (or defaults to 0).This mux is used as an index into the sockets array of the
TinyGsmSim7000
modem to store a pointer to the newly created clientTinyGSM/src/TinyGsmClientSIM7000.h
Line 53 in b381fa5
I don't quite understand why you have to specify the
mux
when onlyTinyGsmSim7000
knows how many clients are already registered. If two methods use the samemux
value then they overwrite the sockets array entry, and the link to one of the client is lost.Wouldn't it be easier to have a factory method
createClient(...)
inTinyGsmSim7000
which will create a newGsmClientSim7000
and register it in the first available sockets[] index or return an error if the array is full?Also, when a
GsmClientSim7000
is destroyed, I don't think its removed from the sockets array, resulting in a memory leak ... or am I missing something maybe?I would be more than happy to create a pull request for this, but before I start coding this I want to make sure I'm not missing the thoughts behind the current implementation that might make this a perfectly valid implementation.
The text was updated successfully, but these errors were encountered: