RoboticsTeam4904 / standard

Command Based Git Submodule
BSD 3-Clause "New" or "Revised" License
11 stars 3 forks source link

can caching issue #142

Open leijurv opened 7 years ago

leijurv commented 7 years ago
public CANSensor(String name, int id, int modes) {
        super(name, id);
        values = new int[modes];
        ages = new long[modes];
        for (int i = 0; i < modes; i++) {
            values[i] = 0; // Should we make this a more obscure value (e.g. 2^32 - 1)?
            ages[i] = System.currentTimeMillis();
        }
    }

Setting ages[i] to System.currentTimeMillis() ends up caching the value 0 at the current time. This is undesired, because the value 0 is garbage and shouldn't be used. Perhaps ages[i] should be set to 0 so there's no way that it could be close enough to the currentTimeMillis to be cached

leijurv commented 7 years ago

Referring to

if (System.currentTimeMillis() - ages[mode] > MAX_AGE) {
            throw new InvalidSensorException("CAN data oudated For CAN sensor " + getName() + " with ID " + messageID);
        }

It's possible that it will cache the garbage 0 value from the beginning because of this snippet

carturn commented 7 years ago

This is true, although I am concerned about setting lastRead (the new version as of #128) to 0, as that would lead to failure if the first read fails regardless (whereas leaving lastRead as the construction time allows for up to 100ms of bad reads at the beginning). It would be optimal to get an initial sensor value and set lastRead with that. There may be the issue of blocking construction for 100ms while we wait for a dead sensor though, which is not good.