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

can caching issue #142

Open
leijurv opened this issue Jan 27, 2017 · 2 comments
Open

can caching issue #142

leijurv opened this issue Jan 27, 2017 · 2 comments

Comments

@leijurv
Copy link
Contributor

leijurv commented Jan 27, 2017

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
Copy link
Contributor Author

leijurv commented Jan 27, 2017

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
Copy link
Contributor

carturn commented Jan 27, 2017

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.

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

No branches or pull requests

2 participants