andrewjfreyer / monitor

Distributed advertisement-based BTLE presence detection reported via mqtt
1.66k stars 194 forks source link

Repetitive scanning (-r) has a long delay and perhaps a bug in the logic #329

Open rccoleman opened 4 years ago

rccoleman commented 4 years ago

I've been using this for the past week or so and I'm trying to to improve the speed of detecting arrivals and departures as much as possible. I've noticed a couple of things in the code that are (in my opinion) unnecessarily delaying scans and could actually be bugs.

First, in the support/btle file, the lescan always runs for the full 60s timeout value before progressing through the remainder of the loop and issuing a "BOFF" to the main_pipe. That's this line:

local hcitool_raw=$(timeout --signal SIGINT 60 hcitool -i $PREF_HCI_DEVICE lescan 2>&1)

Since BOFF is a trigger for the repetitive scan in the main monitor.sh loop (along with BEXP), I typically see repetitive scans delayed by at least 60s while this background scan completes. This has a significant impact on performance, and makes the parameters for the minimum time between arrival and departure scans mostly useless in a quiet environment with few beacons around. The docs make the repetitive scan option (-r) sounds like it's just scanning periodically, but it's still strangely correlated to beacon scans or expirations. I've reduced that timeout from 60s to 10s and it does scan more often, but perhaps making that a customizable value would be helpful.

Second, the logic for repetitive scanning doesn't seem optimal. Specifically, this bit checks to see if we should do a departure scan, which almost always executes because of the long lescan above, and then skips the arrival scan.

                if [ "$duration_since_depart_scan" -gt "$PREF_DEPART_SCAN_INTERVAL" ]; then 

                    perform_departure_scan

                elif [ "$duration_since_arrival_scan" -gt "$PREF_ARRIVE_SCAN_INTERVAL" ]; then 

                    perform_arrival_scan 
                fi

I think that the elif should really be a separate if clause instead, sequentially performing both an arrival and departure scan every time through this loop, rather than doing a departure scan and then skipping past the arrival scan. I've modified it as follows:

                                if [ "$duration_since_depart_scan" -gt "$PREF_DEPART_SCAN_INTERVAL" ]; then

                                        perform_departure_scan
                                fi
                                if [ "$duration_since_arrival_scan" -gt "$PREF_ARRIVE_SCAN_INTERVAL" ]; then

                                        perform_arrival_scan
                                fi

It's possible that there's something about the logic that I'm just not seeing, but making these two changes has significantly improved my scanning performance. I'm coupling this with forced scanning based on doors and motion sensors and can probably extend the lescan time period again, but the reliance on a long background scan and this strange if/elif logic seems odd.

x-dragos commented 4 years ago

i'm having similar issues with no arrival/departures starting after the initial one... i thought it was the bluetooth hardware as i was getting the cycling message sometimes... but apparently if i post to MQTT to start an arrival/depart scan it starts it an has no problems... i am runing only -r start arguments. Maybe this part of the script gets a bit of love, as at the moment, there isnt any easy way to monitor Android devices that dont broadcast BT presence