patrickjmcd commented 6 years ago

If keepAlive > 0, the tag will emit a KeepAlive event every keepAlive seconds.
The value defaults to 0 (keepAlive disabled).

Description, Motivation, and Context

In order to keep values current, implement a KeepAlive event to periodically (but infrequently) update the stored value of the tag based on a keepAlive property of the tag.

How Has This Been Tested?

All tests pass, functional test completed

Types of changes

[ ] Bug fix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

[x] My code follows the code style of this project.
[ ] My change requires a change to the documentation.
[ ] I have updated the documentation accordingly.
[x] I have read the CONTRIBUTING document.
[ ] I have added tests to cover my changes.
[x] All new and existing tests passed.
[ ] This is a work in progress, and I want some feedback (If yes, please mark it in the title -> e.g. [WIP] Some awesome PR title)

Related Issue

15

coveralls commented 6 years ago

Pull Request Test Coverage Report for Build 66

2 of 10 (20.0%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.2%) to 50.14%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/tag/index.js	2	10	20.0%
<!--	Total:	2	10	20.0%	-->

Totals
Change from base Build 61:	-0.2%
Covered Lines:	415
Relevant Lines:	793

💛 - Coveralls

cmseaton42 commented 6 years ago

looks good, just a couple of changes.

Can you clean the input? something like the following could work.

if (typeof keepAlive === "string") 
    throw new Error(`Tag expected keepAlive of type <number> instead got type <${typeof keepAlive}>`);

if (keepAlive < 0 || !Number.isInteger(keepAlive)) 
    throw new Error(`Tag expected keepAlive to be a positive integer, instead got ${keepAlive}`);

Can you add any relavant tests that would be easy (not requiring mocking an endpoint)?

patrickjmcd commented 6 years ago

Sure thing. I'll get to work on that this morning.

I'm thinking I don't need the lines

const { stage_write } = this.state.tag;
if (!stage_write) this.state.tag.value = newValue;

because if a write is staged, the value will be changed, right?