search

vshymanskyy / TinyGSM

A small Arduino library for GSM modules, that just works
GNU Lesser General Public License v3.0
1.95k stars 723 forks source link

Why specify mux when creating GsmClientSim7000 for a TinyGsmSim7000 #623

Closed Bascy closed 2 years ago

Bascy commented 2 years ago

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 client https://github.com/vshymanskyy/TinyGSM/blob/b381fa5c7404650a5b6167414b5c141af26377fb/src/TinyGsmClientSIM7000.h#L53

I don't quite understand why you have to specify the mux when only TinyGsmSim7000 knows how many clients are already registered. If two methods use the same mux 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(...) in TinyGsmSim7000 which will create a new GsmClientSim7000 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.

SRGDamia1 commented 2 years ago

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.