dump1090 icon indicating copy to clipboard operation
dump1090 copied to clipboard

Net cleanups

Open mutability opened this issue 10 years ago • 8 comments

This is moderately invasive; I'm happy to just keep it in my tree if you don't want to merge. (Need it locally mostly for the SBS output speedup - as otherwise my Pi runs out of CPU very fast)

See comments on 1769ac9006df910811d8cf2ce556169035ff6610 for most of the detail

mutability avatar Oct 03 '14 22:10 mutability

Whilst I like the vast majority of this code, and agree it tidies things up, the problem I have is backward compatibility with existing installations - specifically changing the meaning of the --net-ro-size and --net-heartbeat command line switches. There is a lot of "pseudo" documentation on various forums that would be invalidated by these changes.

I think the way around this would be to add new command line switches for 'your' new variables, and then add conversion factors to convert from "rates" to "times" where necessary. That would allow current init.sh scripts and dos batch files to still work.

So I'll file this pull in the "To be looked at again later" pile. Thanks.

MalcolmRobb avatar Oct 29 '14 15:10 MalcolmRobb

There are some merge conflicts that prevent this from applying cleanly to the current codebase. Can you sync your branch with the upstream?

bovine avatar Oct 30 '14 16:10 bovine

I don't know. I am using git hub under protest, and don't understand how a lot of it works. As soon as a merge fails I get in a right mess. My usual resolution is to take copied of both branches, manualy merge the differences (using beyond compare on windows) , and then re-post the result. However, this loses the history of the original poster.

I'm open to suggestions and hints how to do this directly using git-hub tools :-)

MalcolmRobb avatar Oct 30 '14 16:10 MalcolmRobb

Just so I'm clear - I'm talking about your lines 1487 and 1663 being unsafe.

MalcolmRobb avatar Oct 30 '14 16:10 MalcolmRobb

From memory (it's been a while) I tried not to change the meaning of the existing switches - the meaning of the internal values has changed, but the option parsing got correspondingly scaled so the same option value should yield the same behaviour overall.

I'll try to find some time to doublecheck that, and to update this for against the current master.

mutability avatar Oct 30 '14 18:10 mutability

It's conventional to make the person that submitted the pull request to sync their branch by pulling the upstream changes into it (mutability:net-cleanups, in this case) and resolving any merge conflicts themself. They can then push those recommitted conflict fixes to their branch and the pull request will automatically be updated by github to includes those new changes.

bovine avatar Oct 30 '14 18:10 bovine

I merged current master into this branch. Compiles, and I've eyeballed the diff, but not tested.

I stomped on some changes from #33, preferring my code, as I don't believe that those changes are sufficient to avoid list corruption (see my comments on #48)

(Also juggled the EOF detection code around a bit so it wasn't testing the same thing in several places)

mutability avatar Oct 31 '14 18:10 mutability

I looked the compatibility of the options, and I think they're OK.

--net-heartbeat took a value in seconds and it still does now. The default has changed slightly, from approx 57s (900 * 64ms approx) previously to 60s now.

--net-ro-rate previously took a value in 1/15th second units (actually "number of sample buffers" - so it would change if you resized the buffers or changed the sample rate) . It now behaves as if you'd provided --net-ro-interval with a value in seconds (dividing by 15). I removed this option from the help but it's still supported if provided. The only practical change is for values that represent fractional seconds; they now get truncated when converted to seconds. Is it useful to support values that represent fractional-second intervals here? It'd require changes in net_io.c to do that as currently it uses time_t to track time, which has a resolution of 1 second. We'd have to find a fractional representation that both posix & windows were happy with (does windows have gettimeofday or clock_gettime?)

--net-ro-interval is the replacement for --net-ro-rate and takes values in seconds.

--net-ro-size is unchanged, I think. I changed the naming of the internal variables to reflect what it's really setting: the high-water-mark for flushing data to the network. It does not (and did not, previously) control the total buffer size. Is there some change in behaviour you see here?

mutability avatar Oct 31 '14 18:10 mutability