ARMmbed / ble-examples

BLE demos using mbed OS 3 and yotta.
Apache License 2.0
24 stars 27 forks source link

Eddystone example does not check any errors #48

Open andresag01 opened 8 years ago

andresag01 commented 8 years ago

Currently the Eddystone example does not check any errors whatsoever. This makes it very difficult for the user to debug if there is a problem (such as a parameter being wrongly set).

Error checking should be introduced throughout the implementation.

ciarmcom commented 8 years ago

ARM Internal Ref: IOTSFW-1717

scottjenson commented 8 years ago

Actually, the one place is does check is if all 3 packet types have an advertising rate of 0. In that case it currently returns an error. I'd argue that is not an error for the simple reason that we need a software means of turning the beacon 'off'. Not all beacons have on on/off switch.

In addition, every Eddystone config app we've used (and we've used many...) allows this option presumably for the same reason. Our proposed solution is to just not return an error in this specific case.

andresag01 commented 8 years ago

@scottjenson: The reason why this check is there is because we thought that users would want to know that they are trying to broadcast "nothing" (because 0 turns off frames). As you said, perhaps this is not an error.

With regards to your other comment:

we need a software means of turning the beacon 'off'

If what you are actually hoping to accomplish is to stop the Eddystone-URL Configuration service from software there is a function that achieves exactly just that, have a look at stopCurrentService(). Then, when you actually want to start advertising something, you can use startBeaconService().

scottjenson commented 8 years ago

I agree with your comments but I'm thinking ahead to the configuration service, which will allow you to set the ad rate. In this spec, setting a rate to 0 turns it off. It is entirely reasonable that someone, using just the config service will set all three 3 types to 0 in order to turn it off.

andresag01 commented 8 years ago

@scottjenson: Actually, the code that you should have a look at if what you are hoping to is change the advertising rate while in advertising mode is:

Looking closely at these three functions, there is nothing that stops setting the advertisement interval to 0 for all three frames while you are in EDDSYTONE_BEACON_MODE. So, this is probably an inconsistency because the library does not allow you to set the advertising interval for all 3 frames to 0 when starting the advertising but it does AFTER you have started.

As I mentioned before, these issues arise because we have to properly define what is and "error" and what isnt, then implement this in the library.

scottjenson commented 8 years ago

Excellent, thanks for chasing that down. Sounds like we should make this consistent. I'm happy to submit a pull request that removes this error check from startBeaconService()