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

Items with STATES don't respect LIMITS #564

Closed
jasonatball opened this Issue Sep 11, 2017 · 4 comments

Comments

Projects
None yet
2 participants
@jasonatball
Collaborator

jasonatball commented Sep 11, 2017

Items with states defined do not do any limits checking. This has been the default behavior for a long time but it's a bit unexpected. If you define a limits range and then define a single state for a particular error code I wouldn't expect my previously defined limits to not be used.

I think the logic should be to first check states for state colors and then apply limits if no state color was applied. This would allow states to be used in conjunction with LIMITS.

@jasonatball jasonatball added the bug label Sep 11, 2017

@jasonatball

This comment has been minimized.

Show comment
Hide comment
@jasonatball

jasonatball Sep 14, 2017

Collaborator

@ryanatball I checked the code in packet.rb and we check for if item.states and try to find a state key based on the value. If not we run apply_format_string_and_units which is exactly what we do to non-state values. I think an item with STATES should not allow LIMITS, LIMITS_RESPONSE, FORMAT_STRING or UNITS and should return the same string for CONVERTED, FORMATTED, and WITH_UNITS. RAW would return the raw value as usual. Thoughts?

Collaborator

jasonatball commented Sep 14, 2017

@ryanatball I checked the code in packet.rb and we check for if item.states and try to find a state key based on the value. If not we run apply_format_string_and_units which is exactly what we do to non-state values. I think an item with STATES should not allow LIMITS, LIMITS_RESPONSE, FORMAT_STRING or UNITS and should return the same string for CONVERTED, FORMATTED, and WITH_UNITS. RAW would return the raw value as usual. Thoughts?

@ryanatball

This comment has been minimized.

Show comment
Hide comment
@ryanatball

ryanatball Sep 14, 2017

Member

LIMITS_RESPONSE is still valid for colored states.

LIMITS, FORMAT_STRING, and UNITS don't make much sense. Except for the case you mentioned earlier where someone might want to mark one value as ERROR and otherwise have it acting like a numeric.

Member

ryanatball commented Sep 14, 2017

LIMITS_RESPONSE is still valid for colored states.

LIMITS, FORMAT_STRING, and UNITS don't make much sense. Except for the case you mentioned earlier where someone might want to mark one value as ERROR and otherwise have it acting like a numeric.

@jasonatball

This comment has been minimized.

Show comment
Hide comment
@jasonatball

jasonatball Sep 14, 2017

Collaborator

If it has any states it could sometimes come back as a string and thus we should probably format it as a string all the time. Otherwise you wouldn't be able to handle it in a database. So maybe we just take a hard line and say no LIMITS with STATES. If you really want limits then apply the limits. If you have some weird state value you want to flag then use a derived telemetry point which reads the value and applies the state.

Collaborator

jasonatball commented Sep 14, 2017

If it has any states it could sometimes come back as a string and thus we should probably format it as a string all the time. Otherwise you wouldn't be able to handle it in a database. So maybe we just take a hard line and say no LIMITS with STATES. If you really want limits then apply the limits. If you have some weird state value you want to flag then use a derived telemetry point which reads the value and applies the state.

@ryanatball

This comment has been minimized.

Show comment
Hide comment
@ryanatball

ryanatball Sep 14, 2017

Member

It really is a bad thing to mix states with an item that is mostly not states. I wish we could prevent that better but the problem is we don't know how they are going to use the values.

Member

ryanatball commented Sep 14, 2017

It really is a bad thing to mix states with an item that is mostly not states. I wish we could prevent that better but the problem is we don't know how they are going to use the values.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment