Skip to content
This repository has been archived by the owner on Feb 4, 2025. It is now read-only.

Shutdown bug fix, and a frequency source selection feature #55

Merged
merged 7 commits into from
Jan 25, 2021
Merged

Shutdown bug fix, and a frequency source selection feature #55

merged 7 commits into from
Jan 25, 2021
+29 −10

Conversation

jcsmook
Copy link
Contributor

@jcsmook jcsmook commented Jan 22, 2021

  • Using WinNUT for the first time, I had a issue where everything seemed to work, but my Windows PC did not Shutdown when it was supposed to. The issue turned out te be a settings issue. As WinNUT is running on a headless machine, I would not use the grace period, and therefore left the parameters at the default values. A setting of 0 for the Grace delay caused an exception when setting a timer interval, causing the initialization sub to fail. This intern prevented any timer from starting, including the Shutdown timer.

  • I also stumbled on a bug where the connection would Force Disconnect if the LOAD parameter is reported as 0. This caused a infinite re-connect loop, until the load changes to non-zero.

  • Added a feature to for the user to select the source of the Frequency used in the display (Input/Output). My UPS has no Input Frequency available, but does report the Output frequency.

@jcsmook
Force Disconnect bug
640a81b 
Battery runtime calculation can cause an continous Forced Disconnect.

- If Load is 0, the calculation results in infinity. This causes an exception in DataUpdated() when calculating the timespan, causing Me.Disconnect (true) to run in the exception handler

Thus, a connection is established, but is forcefully disconneced almost immediately. This cycle repeats on each connect until load is <> 0
@jcsmook
Bug causing a No Shutdown Action
68dc845 
This exception halts the whole Shutdown gui initialization, causing a failure to apply the shutdown action.

Cause: If the ExtendedShutdownDelay value is the default value of 0, it causes an exception when setting the Grace Timer interval (interval can't be 0).

Solution (Not Complete): Moved the setting of the Grace Timer interval only if Grace Interval is used.

(This error will happen as well if user sets the DelayToShutdown parameter as 0)
@jcsmook
Toast update
5cc28a3 
1) ToastPopup.vb: An exception is thrown every time a toast with 4 parameters is requested, i.e.
  WinNUT - 2.0.5.0
  Connected
  On Line
  Battery OK

This causes an exception as the ToastText02 is used. The For loop was also changed to run to the least of the 2 counts, not the most.

2) WinNUT.vb: The raisevent happens before the UpdateNotifyIconStr function, causing the Toast generated to use the old data contained in Me.NotifyIcon.Text.

Moved RaiseEvent to after String update. (I have not tested this fix properly)
@jcsmook
Toast update 2
8f4ad26 
Correction -> I forgot to remove the old RaiseEvent lines from the begining of the Sub.
@jcsmook
Update Pref_Gui.vb
a79350e 
Prevent varibles from being set to values that will cause exceptions due to Timer.Interval, in Shutdown_Gui.vb
@jcsmook
Added User selectable Frequency source
985d9cb 
With My UPS, only an output frequency reading is available. I presume they did in this manner as the output frequency is the same as the input when in line mode (only routed to output), and if it is using the battery, only the inverter output frequency is available.

So I have added a combo box to select the Input/Output as the source. This causes the text above the Frequency guage to change, and the correct reading to be retrieved from the UPS Network. The new field was added to the registry Parameters as well.

I added 2 strings to the settings resource to help with the translation of the guage heading label. I am sure there is a better way...

If possible, please check the translations. I resized the input controls on the calibration tab in the preferences form, so there ara quite a few changes in the language file.
@gawindx
Copy link
Owner

gawindx commented Jan 24, 2021

Well,

Already, thank you for your interest in my work, it's always nice.

But, I have several problems with your request:

  • Following the last code updates, I have a conflict with the "Added User selectable Frequency source" commit
  • On this same commit, I do not entirely agree with the method.

Let me explain, the fact of choosing whether it is the input frequency or the output frequency is not intuitive and complicates the configuration (which I find is already enough like that).

Wouldn't it be easier to change the fallback_value to take the output frequency if the input frequency is incorrect?

Apart from that, everything else seems correct to me and if there was not this problem, I would have validated it without hesitation.

Can you delete this commit from the request so that this part already validates and we see in a second step for this functionality.

The idea would be to release v2.0.5 with this feature.

@jcsmook
Copy link
Contributor Author

jcsmook commented Jan 25, 2021

Morning gawindx,

Thanks, it is a great tool you gave us!

The only reason I did not just add that to the fallback value, was that there is no fallback on the fallback (If no input or output frequency is available, 0 (I presume the value would be 0) always looks like a problem.

You know your program and user base better than anyone, so I will remove the last commit, and if you want the code for the last commit, I can do a new pull request, or send it to you - let me know.

Enjoy, and thanks again for the great app.

@gawindx gawindx merged commit a7768f8 into gawindx:master Jan 25, 2021
@gawindx
Copy link
Owner

gawindx commented Jan 25, 2021

Just to see how we can solve this frequency problem, can you paste all the variables of your UPS to me?

In Winnut, "File" menu and "List UPS Variable's".
You can stick the whole thing in the clipboard.

@jcsmook
Copy link
Contributor Author

jcsmook commented Jan 26, 2021 via email

@gawindx
Copy link
Owner

gawindx commented Jan 26, 2021

yes, it can work
either the input frequency is provided, or we can take the output frequency and as a last resort we fall back on the theoretical value configured via the parameters.

for the gauge, the most sumple would be to rename without specifying whether it is input or output, as that is always correct (well after, if we are on the fallback value it will be purely aesthetic)

I see what I can do during the week and I come back to you

@gawindx
Copy link
Owner

gawindx commented Jan 27, 2021

I updated the code and made some changes for the frequency.
If you can test what I have changed and say if that fixes your problem.

@jcsmook
Copy link
Contributor Author

jcsmook commented Jan 27, 2021 via email

@gawindx
Copy link
Owner

gawindx commented Jan 28, 2021 •
edited
Loading

I don't know if you've tried it yet, but I made another change:

  • First, the fallback value is determined, which is either the value of the "output.frequency.nominal" property, or the value defined in the parameters (50 or 60Hz).
    The variable is now defined in Static and we check if it is initialized (other than zero) so as not to recalculate it each time.
  • When recovering the value of the frequency, we recover the value of "input.frequency" to which we apply the fallback value of " output.frequency" to which we applied the value calculated at the previous point.

To summarize :
input.frequency or output.frequency or output.frequency.nominal or frequency defined in the parameters.

I think this should cover all cases.

@jcsmook
Copy link
Contributor Author

jcsmook commented Jan 29, 2021 via email

gawindx added a commit that referenced this pull request Feb 5, 2021
@gawindx
###Version 2.0.7706
2934356 
###Version 2.0.7706
####Fix :
  - App crash when disconnecting with null streams (Thanks to tgp1994) (Issue #48)
  - Correction of a bad state of the Notify Icon Text when the return value of ups status is not only "OL" (Issue #45)
  - Add -f parameter to force shutdown (Issue #46)
  - Some modifications provided by tgp1994
  - Fixed a bug that could occur when using a value of "0" for the grace period (thanks to jcsmook - pull requests #55)
  - Fixed a bug generating an infinite connection / reconnection loop when the load value is retrieved to "0" during connection (thanks to jcsmook, pull requests # 55)
  - Correction of a code error causing the event time not to be updated in the logs.
  - Fixed an error causing an unhandled exception when you do not want to apply the update immediately and the installation file already exists at the location specified for saving.
  - In the case of a left click on the systray icon, WinNUT was restored to its original size and the context menu was opened at the same time.
  This behavior has been corrected and only the context menu opens.
  Restoring the WinNUT window to its original size is caused by a single or double click only.

####Added :
  - Addition of a directory containing the translations at the repository level.
	Makes it easier to submit a new translation (or correction) via a fork / pull request (nonPointer idea - issue #35)
  - Translation of the "List UPS Variable's" interface (translation forgot during v2.0)

####Changed :
  - Some changes on how logs work (Thanks to tgp1994)
  - Modification of the code to recover the power supply frequency of the UPS when it only provides the output frequency (modification made in response to an unsuccessful commit of pull requests # 55 - problem encountered by jcsmook)
  - Modification of the generated installer:
		- Addition of a custom image banner
		- Removal of the dependent Windows libraries installed in the WinNUT directory
  - The update process is no longer based on the "changelog.txt" of the repository but on the list of releases via the GitHub APIs.
  This modification has been implemented to avoid reproducing issue #53.
  This modification also brings the possibility of receiving both stable and development updates for those who choose to follow the development channel (the more recent of the two being the proposed update).
  - The changes made to the update process allow the implementation of a more suitable versioning of the type
    [Major Version].[Minor Version].[Automatic Build Version].[Automatic Revision Version]
  - Complete syntax review of Changelog.txt file
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jcsmook @gawindx