Shouldn't BeaconParser identifier be part of Region?

[Whathecode]

It took my quite some time to figure out how BeaconParser fits into the picture. As a consumer of the API, it is extremely unclear how this works, or how it is related to the Region class, without digging deeper into the code (i.e., the API has failed).

I now understand BeaconManager can register multiple BeaconParsers, which is exposed as a mutable list (a first potential design flaw). By default, there is one parser registered for AltBeacon layouts.

But, which one is used for callbacks to MonitorNotifier is completely opaque. Apparently, it depends on the m setting of the layout of registered BeaconParsers. But, you could imagine scenarios in which multiple layouts could match; another design flaw.

At the same time, when initializing Region objects for scanning, I need to be fully aware about which BeaconParser's are configured, and how these BeaconParser's map to Identifiers in Region. There is thus only a false sense of decoupling parsers from scanning here.

At a glance, I think doing the following would improve the API:

• At the very least, make sure that all callbacks know which parser was used. RangeNotifier apparently returns Beacons which do carry parserIdentifier, but MonitorNotifier does not.
• Possibly consider making Region polymorphic. The base type can do an internal mapping to the current List<Identifier>, but the API should only expose proper initializers. E.g., AltBeaconRegion(manufacturerId, organizationId, majorId, minorId). These constructors could be strongly-typed, encapsulating which Identifier factory method to use.
• You could even automatically register parsers upon scanning for unknown regions if the layout is statically linked to these concrete Region classes, i.e., expose a getLayout in their interface.

[davidgyoung]

@Whathecode these are good ideas, thank you. I particularly like the idea of having Region include a type that can match a particular BeaconParser. Since this is a breaking change I am marking it for the 3.0 release.

There are historical and legal reasons that we cannot have polymorphic regions, at least for the most popular beacon format. The company that owns this format limits how third parties can use it directly on Android. And I hate to add polymorphic reagions for the less popular formats and not be able to have one for the most popular format!

Part of the reason you had trouble with the concept of a BeaconParser is that lawyers won't allow us to make the documentation more explicit for the most popular use case. This library is designed to be flexible enough to allow developers to easily configure the most popular format themselves without having the specifics of that beacon format built-in to the library.

[davidgyoung]

Moved discussion to #1052