itm / wsn-device-drivers

Drivers for Wireless Sensor Network Devices
Other
6 stars 4 forks source link

Why do we need a dedicated method getChannels()? #36

Closed mlegenhausen closed 13 years ago

mlegenhausen commented 13 years ago

This method is equals for the jennic and pacemate devices and TelosB returns null. So when this data is needed it should be provided at a higher level?

ghost commented 13 years ago

Don't know if anybody uses this method (WISEBED Node API?). The supported channels depend on the build in radio chip. For example the Waspmotes use so called XBeePros and they support only a subset of the 802.15.4 channels. So the method makes sense at device or device type level but I don't know if it is accessible via Testbed Runtime or an relict of the old iShell drivers.

danbim commented 13 years ago

Its not accessible via Testbed Runtime but I guess it makes sense if we use the drivers outside of TR (what we're actually already doing). So I would keep the method. I'm not sure though, how the issue is meant. Is it just a question of where to actually place the code or if we need it in general?

mlegenhausen commented 13 years ago

That was the question if it is anywhere used or simply deprecated. Cause inside the drivers it is never used. If it is needed it is the question where to place the code. When I want to remove the Device class it has to be placed somewhere else. Currently I thinking the array can simply injected in the DeviceAsync and accessed by the getter.

danbim commented 13 years ago

I would keep it as part of the drivers in every case as I'm sure it will be used somewhen and it's the right place for it (although one could provide a more general meta-data thingy, but let's not over-engineer). If DeviceAsync will be the only interface in the future we can surely place it there.

What do you mean by injecting this information? Isn't this overkill? It feels like this should be a constant array depending on what class you're in as it's constant for a given device type.

mlegenhausen commented 13 years ago

It isn't ;) One line in the Guice Module, one named annotation for the constructor argument of the DeviceAsync or I would place it in the connection, which would make sense too, cause when no connection is available no channels are needed ;)

danbim commented 13 years ago

Can you give a me a use-case where this injection would be used?

mlegenhausen commented 13 years ago

When you call the getChannels() method? This is the case when you want to use the jennic or pacemate devices.

Example (I think I can't inject primitives and arrays?):

public class QueuedDeviceAsync implements DeviceAsync {
    private List<Integer> channels;

    @Inject
    public QueuedDeviceAsync(OperationQueue queue, Device<? extends Connection> device, @Named("channels") List<Integer> channels) {
        this.channels = channels;
    }

    public int[] getChannels() {
        return channels.toArray(new int[0]);
    }
}

In the Guice Module:

bind(List.class).annotatedWith(Names.named("channels")).toInstance(Arrays.asList(11, 12, 13));

Or without injection copy and paste the method from the Device to the Connection.

danbim commented 13 years ago

OK. Let me ask differently: what the advantag of DI over static code for this very special case? When would I want to inject different channels?

mlegenhausen commented 13 years ago

If you ever want to change the channels for a Device I would place it via DI else as static code in the connection.

danbim commented 13 years ago

When would I want to do that?

mlegenhausen commented 13 years ago

I don't know I have not invented the getChannels() method ;)

danbim commented 13 years ago

So I propose to not overengineer by injecting something of which we cannot find a use case for injection. The method itself makes sense but is totally static in behaviour as one driver class should correspond to one (never-changing) hardware revision.