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

BUG Fix for logging values to a text file #168

Merged
merged 4 commits into from
Mar 30, 2021
Merged

BUG Fix for logging values to a text file #168

merged 4 commits into from
Mar 30, 2021

Conversation

GitHubDragonFly
Copy link
Contributor

Short description of change

BUG Fix

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • I have read the docs/CONTRIBUTING.md document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read tests/README.md.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

What is the change?

  • This fixes a BUG when logging values to a text file (which was introduced with the latest new feature of 81_simple_gui.py file)
  • Some other changes to ease importing the log into Excel (including array values). This has to be done by a user, either change the extension ".txt" to ".csv" in the code or just rename the log file once created.
  • Removed unused variable

What does it fix/add?

Fixes a BUG that caused the log file entries be scattered.

Test Configuration

  • PLC Model
  • PLC Firmware
  • pylogix version 0.7.10
  • python version 2.7.18 and 3.6.8
  • OS type and version Windows 10 v2004

- This fixes a BUG when logging values to a text file (which was introduced with the latest new feature of 81_simple_gui.py file) 
- Some other changes to allow the log file be renamed with the ".csv" extension and directly imported into Excel (including array values)
@TheFern2
Copy link
Collaborator

What is the reason to create a csv data file, and having the extension as txt? Would it make sense to just create a csv from the start?

Not sure if this is what you intended but header row now only has date column:

Date / Time, 
2021-03-30/06:47:33.725992, True, 545437493
2021-03-30/06:47:34.231560, True, 545437493
2021-03-30/06:47:34.739083, True, 545437493
2021-03-30/06:47:35.244650, True, 545437493

@GitHubDragonFly
Copy link
Contributor Author

The txt extension is generally acceptable for everybody. Some users might not be using csv at all and could possibly get confused with the extension.

The log header should be fixed now.

@GitHubDragonFly
Copy link
Contributor Author

GitHubDragonFly commented Mar 30, 2021

Logging values only makes sense for the initial list of tags being logged.

If the tags are changed then the current log file should be renamed or moved to a different location.
Otherwise, the newly logged values will not match the existing header.

@TheFern2 TheFern2 merged commit 3a35c55 into dmroeder:master Mar 30, 2021
@TheFern2
Copy link
Collaborator

Makes, sense. Thanks for the bug fix.

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

Successfully merging this pull request may close these issues.

2 participants