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.

Sign up for GitHub

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

Jump to bottom

Add FLOAT64-IEEE support to inputs.modbus (#8361) (by @Nemecsek) #8474

Merged
merged 6 commits into from
Nov 30, 2020
Merged

Add FLOAT64-IEEE support to inputs.modbus (#8361) (by @Nemecsek) #8474

merged 6 commits into from
Nov 30, 2020

Conversation

soerman
Copy link
Contributor

@soerman soerman commented Nov 25, 2020 •
edited
Loading

This adds FLOAT64-IEEE support (issue #8361).
Please note that @Nemecsek is the original author → See discussion in #8361.

Required for all PRs:

  • Signed CLA.
  • Associated README.md updated.
  • Has appropriate unit tests.

@soerman soerman mentioned this pull request Nov 25, 2020
Closed
@garciaolais
Copy link
Contributor

@soerman please consider adding some test cases for this new type

@sjwang90 sjwang90 added area/iot New plugins or features relating to IoT monitoring area/modbus feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin labels Nov 25, 2020
@srebhan srebhan self-assigned this Nov 25, 2020
@srebhan
Copy link
Member

srebhan commented Nov 25, 2020 •
edited
Loading

@soerman: We also need the description in the README.md file. Futhermore, you need a CLA otherwise we cannot merge this PR...

ssoroka
ssoroka suggested changes Nov 25, 2020
View reviewed changes
Copy link
Contributor

@ssoroka ssoroka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, we should have some test cases to avoid any regressions

@soerman soerman requested a review from ssoroka November 28, 2020 18:16
@youcann
Copy link

youcann commented Nov 28, 2020

Seems good now

srebhan
srebhan approved these changes Nov 30, 2020
View reviewed changes
Copy link
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@ssoroka ssoroka merged commit 0537898 into influxdata:master Nov 30, 2020
arstercz pushed a commit to arstercz/telegraf that referenced this pull request Mar 5, 2023
@soerman
Add FLOAT64-IEEE support to inputs.modbus (influxdata#8361) (by @Neme…
20806ee 
…csek) (influxdata#8474)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@srebhan srebhan srebhan approved these changes

@ssoroka ssoroka ssoroka approved these changes

Assignees

@srebhan srebhan

Labels
area/iot New plugins or features relating to IoT monitoring area/modbus feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@soerman @garciaolais @srebhan @youcann @ssoroka @sjwang90