Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a flightradar24 client. #123

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

quentinmit
Copy link
Contributor

This allows flightradar24 to be a telemetry source. There wasn't any way to get access to the TelemetryStore for things other than IDemodulators, so I had to add a hook for device components to get access. PTAL and feel free to suggest a better way to hook it in.

@@ -138,6 +138,12 @@ def close():
__all__.append('IComponent')


class ITopWatcher(Interface):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a good plan. Top is not a well-designed interface, and is going to go away in its current form when the multi-session refactoring finally happens.

Since I want this to happen anyway, I'll see if I can implement a better approach for hooking up telemetry.

Copy link
Owner

@kpreid kpreid Dec 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Designed and implemented the better interface. Commit 53f699e adds a DeviceContext which can be used for this purpose, and commit 76afa0a is an example of it being used for the existing APRSISRXDevice.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Done.

self.__top = top

def __make_url(self):
u = URL.fromText('https://data-live.flightradar24.com/zones/fcgi/feed.js?faa=1&mlat=1&flarm=1&adsb=1&gnd=0&air=1&vehicles=0&estimated=1&maxage=14400&gliders=1&stats=0')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make this a parameter passed in from the Flightradar24 function so that it can be replaced with a local stub for testing, or if the user wants to change some parameters.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Owner

@kpreid kpreid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, I keep forgetting to push 'Start a review' on GitHub. Sorry for the disorganized comments.

I like this feature addition! There's already a half-baked one that is structurally similar — APRSISRXDevice in the APRS plugin. It has the same problem with accessing the telemetry store that you had, so a solution should be applied to it too.



@implementer(IAircraft, ITelemetryObject)
class Aircraft(ExportedState):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should share an Aircraft telemetry object type with the Mode S plugin so that information from both sources can be combined and not duplicated (and we don't need to duplicate the map layer code too).

There's already a multi-plugin situation in APRS — there, there is a plugin for core APRS functionality and a plugin for the multimon-ng decoder. Here, I'd suggest for now depending on the one in the Mode S plugin. If you need to make any changes to it, add comments there that it's used in two places.

The logic to decode the two kinds of messages can go in the message objects.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I agree it should share the same Aircraft layer as the Mode S plugin. Ideally I would like Flightradar24 to be its own map layer, and to have the enable/disable checkbox on the layer actually toggle the enablement of the event loop here.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Separate layers can be handled by adding something like a "from what data sources" field to the objects and having the layer provide subordinate filtering options (much like the database layer lets you enter search text). That gets you the functionality without duplicating code and preventing data merging.

(Now that I've thought of it, it seems reasonable to have 'data sources for this object' be a general feature of the telemetry system; e.g. someone might want to filter APRS the same way.)

_FEET_PER_MINUTE_TO_METERS_PER_SECOND = _METERS_PER_FEET * 60


# TODO: This really shouldn't be a Device, but that's the only way right now to hook into the config.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, being a Device is fine and correct, since it connects to something in the outside world and receives information. Rather, we should improve support for non-RF devices. (Delete this comment.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

This renders aircraft from flightradar24 much like the mode_s plugin.
This dramatically improves performance of the map with many aircraft
by reducing the number of cells per aircraft.
Copy link
Contributor Author

@quentinmit quentinmit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay, I've been very busy over the holidays. PTAL.

@@ -138,6 +138,12 @@ def close():
__all__.append('IComponent')


class ITopWatcher(Interface):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Done.

_FEET_PER_MINUTE_TO_METERS_PER_SECOND = _METERS_PER_FEET * 60


# TODO: This really shouldn't be a Device, but that's the only way right now to hook into the config.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.



@implementer(IAircraft, ITelemetryObject)
class Aircraft(ExportedState):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I agree it should share the same Aircraft layer as the Mode S plugin. Ideally I would like Flightradar24 to be its own map layer, and to have the enable/disable checkbox on the layer actually toggle the enablement of the event loop here.

self.__top = top

def __make_url(self):
u = URL.fromText('https://data-live.flightradar24.com/zones/fcgi/feed.js?faa=1&mlat=1&flarm=1&adsb=1&gnd=0&air=1&vehicles=0&estimated=1&maxage=14400&gliders=1&stats=0')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants