scanning for elrs network devices with crsf msp frame capability #2922
Conversation
|
I've mentioned this in Discord but I'll drop it here for the Betaflight folks' info. This should not be using the hostname as the key (which can change). It should use the mDNS TXT records returned from the query |
Yes, I will implement that once we sorted AP mode out 👍 |
src/js/port_handler.js
Outdated
| self.elrs_lock = false; | ||
| } | ||
| }); | ||
| }, 2000); |
There was a problem hiding this comment.
Seems rather high. This function is already called every 500ms. So it has to fit into this time window.
There was a problem hiding this comment.
I don't want to flood the network. My second idea was to enable AP mode scanning my hand and don't have it on all the time. How do you think about that?
There was a problem hiding this comment.
That might be a better approach.
src/js/port_handler.js
Outdated
| self.elrs_lock = true; | ||
|
|
||
| setTimeout(() => { | ||
| ping.sys.probe('10.0.0.1', function (isAlive) { |
There was a problem hiding this comment.
Should define a constant for 10.0.0.1
|
AUTOMERGE: (FAIL)
|
|
While I can't vouch for anything regarding the appropriateness of how this fits into the configurator's scanning pattern, I can say that the ELRS detection from the mDNS responses gets my full approval. This old incorrect comment should be removed though since it no longer does this |
|
i refactored to another package which is now event based, so no timer anymore. With the current ELRS master, the need of pinging a constant is obsolete. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Kudos, SonarCloud Quality Gate passed!
|
This comment has been minimized.
This comment has been minimized.
|
expressLRS v3 RC is approaching rapidly, would be good to get this included in BF configurator so WiFi connection can be tested easily. |
|
@freasy please rebase and squash |
freasy
force-pushed
the
feature/mdns_check_and_resolve
branch
3 times, most recently
from
425f98f
to
66521c3
Compare
i removed the lock file
i rebased current master and squashed to 1 commit the android part still needs testing - i couldnt do the test, because remote chrome debug wasnt catching the bf app's chromium |
This comment has been minimized.
This comment has been minimized.
|
@KarateBrot @daleckystepan wana has a look at this |
|
Checked out the branch but it's not working (Hangs at loading as PortHandler can't be initialized) |
src/js/port_handler.js
Outdated
| @@ -44,6 +142,10 @@ PortHandler.check = function () { | |||
| self.check_serial_devices(); | |||
| } | |||
|
|
|||
| self.check_elrs_devices(); | |||
There was a problem hiding this comment.
Where is this function defined?
There was a problem hiding this comment.
Making this changes restores PortHandler to normal operation. Did not test ELRS functionality:
diff --git a/src/js/port_handler.js b/src/js/port_handler.js
index a9230135..dab965bb 100644
--- a/src/js/port_handler.js
+++ b/src/js/port_handler.js
@@ -1,40 +1,5 @@
'use strict';
-const http = require("http");
-const bonjour = require('bonjour')();
-
-const MDNS_INTERVAL = 10000;
-const TCP_CHECK_INTERVAL = 5000;
-const TCP_TIMEOUT = 2000;
-
-let mdnsBrowser = null;
-if (GUI.isCordova()) {
- mdnsBrowser = {
- services: [],
- };
-
- const zeroconf = cordova.plugins.zeroconf;
- zeroconf.registerAddressFamily = 'ipv4'; // or 'ipv6' ('any' by default)
- zeroconf.watchAddressFamily = 'ipv4'; // or 'ipv6' ('any' by default)
- zeroconf.watch("_http._tcp.", "local.").subscribe(result => {
- const action = result.action;
- const service = result.service;
- if (['added', 'resolved'].includes(action)) {
- console.log("Zeroconf Service Changed", service);
- mdnsBrowser.services.push({
- addresses: service.ipv4Addresses,
- txt: service.txtRecord,
- fqdn: service.hostname,
- });
- } else {
- console.log("Zeroconf Service Removed", service);
- mdnsBrowser.services = mdnsBrowser.services.filter(s => s.fqdn !== service.hostname);
- }
- });
-} else {
- bonjour.find({ type: 'http' });
-}
-
const TIMEOUT_CHECK = 500 ; // With 250 it seems that it produces a memory leak and slowdown in some versions, reason unknown
const usbDevices = { filters: [
@@ -50,6 +15,7 @@ const PortHandler = new function () {
this.port_available = false;
this.showAllSerialDevices = false;
this.showVirtualMode = false;
+ this.mdnsBrowser = null;
};
PortHandler.initialize = function () {
@@ -66,7 +32,41 @@ PortHandler.initialize = function () {
const self = this;
- mdnsBrowser?.on('down', function (service) {
+ const http = require("http");
+ const bonjour = require('bonjour')();
+
+ const MDNS_INTERVAL = 10000;
+ const TCP_CHECK_INTERVAL = 5000;
+ const TCP_TIMEOUT = 2000;
+
+ if (GUI.isCordova()) {
+ self.mdnsBrowser = {
+ services: [],
+ };
+
+ const zeroconf = cordova.plugins.zeroconf;
+ zeroconf.registerAddressFamily = 'ipv4'; // or 'ipv6' ('any' by default)
+ zeroconf.watchAddressFamily = 'ipv4'; // or 'ipv6' ('any' by default)
+ zeroconf.watch("_http._tcp.", "local.").subscribe(result => {
+ const action = result.action;
+ const service = result.service;
+ if (['added', 'resolved'].includes(action)) {
+ console.log("Zeroconf Service Changed", service);
+ self.mdnsBrowser.services.push({
+ addresses: service.ipv4Addresses,
+ txt: service.txtRecord,
+ fqdn: service.hostname,
+ });
+ } else {
+ console.log("Zeroconf Service Removed", service);
+ self.mdnsBrowser.services = self.mdnsBrowser.services.filter(s => s.fqdn !== service.hostname);
+ }
+ });
+ } else {
+ bonjour.find({ type: 'http' });
+ }
+
+ self.mdnsBrowser?.on('down', function (service) {
console.log('Lost an HTTP server:', service);
let currentPorts = [];
if (self.initialPorts && self.initialPorts.length > 0) {
@@ -86,10 +86,10 @@ PortHandler.initialize = function () {
const tcpPorts = self.initialPorts.filter(p => p.path.startsWith('tcp://'));
tcpPorts.forEach(function (port) {
const removePort = () => {
- mdnsBrowser?._removeService(port.fqdn);
+ self.mdnsBrowser?._removeService(port.fqdn);
if (GUI.isCordova()) {
- mdnsBrowser.services = mdnsBrowser.services.filter(s => s.fqdn !== port.fqdn);
+ self.mdnsBrowser.services = self.mdnsBrowser.services.filter(s => s.fqdn !== port.fqdn);
}
};
http.get({
@@ -123,7 +123,7 @@ PortHandler.initialize = function () {
self.mdns_timer = setInterval(() => {
if (!GUI.connected_to) {
- mdnsBrowser?.update();
+ self.mdnsBrowser?.update();
}
}, MDNS_INTERVAL);
@@ -158,7 +158,7 @@ PortHandler.check_serial_devices = function () {
let currentPorts = [
...cp,
- ...(mdnsBrowser?.services?.filter(s => s.txt.vendor === 'elrs' && s.txt.type === 'rx')
+ ...(self.mdnsBrowser?.services?.filter(s => s.txt.vendor === 'elrs' && s.txt.type === 'rx')
.map(s => s.addresses.map(a => ({
path: `tcp://${a}`,
displayName: `${s.txt.target} - ${s.txt.version}`,
66521c3
to
178b78f
Compare
This comment has been minimized.
This comment has been minimized.
As suggested Co-authored-by: haslinghuis <mark@numloq.nl>
freasy
requested review from
haslinghuis and
chmelevskij
and removed request for
limonspb and
haslinghuis
|
Kudos, SonarCloud Quality Gate passed!
|
|
Do you want to test this code? Here you have an automated build: |
| const action = result.action; | ||
| const service = result.service; | ||
| if (['added', 'resolved'].includes(action)) { | ||
| console.log("Zeroconf Service Changed", service); |
There was a problem hiding this comment.
Would any of these logs be useful for the UI layer? E.G. https://github.com/betaflight/betaflight-configurator/blob/master/src/js/msp/MSPHelper.js#L2940
There was a problem hiding this comment.
totaly, i will change it, so that those logs go to the GUI as well.
There was a problem hiding this comment.
Please change in follow up patch.
haslinghuis
moved this from Configurator
to Ready to merge
in Finalizing Firmware 4.4 Release
|
Tested on Android and it works. For some reason it wouldn't show the FC on the first launch but after Force-Stopping and restarting the app it's working so perhaps it just takes a second to do the mDNS lookup. |
yes mDNS is a b**** specially on android, so if it works sometimes, thats already a big win! I am glad, that it works, since i could not test the code myself. |
|
Don't know if you're looking for more "Me too"s, but I just installed the artifact on my Samsung Galaxy S10e and it discovered my ELRS RX no problem on my home wifi network, then switched over to put the RX into AP mode and it discovered properly on that network as well. My crap eyes thank you for not making me mistype tcp://10.0.01 by mistake 100x before realizing the gray on gray text is missing a pixel! |
haslinghuis
moved this from Ready to merge
to Configurator
in Finalizing Firmware 4.4 Release
haslinghuis
moved this from Configurator
to Ready to merge
in Finalizing Firmware 4.4 Release
|
@freasy I tested it ony lastest nightlies and the autobuild from this PR, it's working on my phone but not on my pc. I'm using win11. The tcp port is showing up for 1-2 sec but after it dissapears. I can connect to it manually but the auto detection is broken. This is the error message: It happens after the TCP_CHECK_INTERVAL expired and it checks again. |
In future ELRS there is CRSF MSP support.
This contains 2 parts because a elrs rx can connect to home wifi or start a access point mode
in this mode the elrs rx responds to mdns requests and sends 3 responses (A, TXT, SRV), with those i can make a valid tcp port
in the latest ELRS master - the mdns is fixed, so i rely on that there too
To check if the connection is still valid we have a "down" event from the mdns browser, but thats not realy reliable, so i poll if a http connection can be made every 5 seconds, this check is skipped, when the GUI is connected to a device.