bertrandmartel / speed-test-lib

:cloud: JSpeedTest : speed test client library for Java/Android
MIT License
381 stars 119 forks source link

Socket constructors & params #39

Closed DimaDDM closed 7 years ago

DimaDDM commented 7 years ago

Now SpeedTestSocket has empty constructor and default update report time == -1. I think param for update report interval should be moved to constructor from startDownloadRepeat and startUploadRepeat btw default should be set as 1000 for example(for empty constructor). Thanks foк your work and updated!

bertrandmartel commented 7 years ago

In fact, if no value is specified, the default value is not -1 but the default value is defined when a given amount of bytes is written (for upload) or read (for download) from the socket. For upload it is set by READ_BUFFER_SIZE which default to 65535 so each time 65535 bytes are written an onProgress callback is called if not value is specified in constructor. For download, it is the same but the number of bytes written is up to 65535. This is the default behavior of the report interval when no value is set by default.

default should be set as 1000 for example(for empty constructor)

I think it's better to send an onProgress by default when a given amount of bytes is read/written, some people may want a smaller value for the report interval, some people may want a higher value. For instance, on a 4G connection 1000 ms is very high if you download 1Mo, you probably won't see any progress callback.

I think param for update report interval should be moved to constructor from startDownloadRepeat and startUploadRepeat

Do you mean you want to have a startDownloadRepeat and startUploadRepeat method without the reportPeriodMillis ?

// set the report interval
public void startDownloadRepeat(
            final String uri,
            final int repeatWindow,
            final int reportPeriodMillis,
            final IRepeatListener repeatListener) 

// take the default report interval (from constructor otherwise default to 1000) ?
public void startDownloadRepeat(
            final String uri,
            final int repeatWindow,
            final IRepeatListener repeatListener) 
DimaDDM commented 7 years ago

Yes, reportPeriodMillis