Thread Rating:
  • 0 Vote(s) - 0 Average
  • 1
  • 2
  • 3
  • 4
  • 5
Monthly Adjust per Station
#1
Hi,

I have more or less completed the third of the 4 plugins I had planned - as the name suggests it allows altering the water amount per station and per month.

   


I have a couple of issues though:

1. The value of the "station" argument does not seem to be correct for the signal "stations_scheduled" as it has the value "SIP" instead of the station number n:
Code:
def notify_station_completed(station, **kw):
    print(f"Station {station} run completd")

completed_signal = signal("station_completed")
completed_signal.connect(notify_station_completed)


def notify_stations_scheduled(station, **kw):
    print(f"Station {station} run scheduled")

scheduled_signal = signal("stations_scheduled")
scheduled_signal.connect(notify_stations_scheduled)


Output:
Station SIP run scheduled
Station 1 run completd


2. The plugin should not affect "Run Now" schedules i.e. schedule started due to manual actions. As is the case with the Moisture Sensor Control plugin however, there is no easy way to identify theses schedules

3. Not really a problem but an inconsistency. The duration for "Run Now" schedules is a float as opposed to an int for a schedule for automatically started programs
Code:
Run Now == Before [[0, 0, 0, 0], [0, 0, 0, 0], [0, 0, 0, 0], [0, 0, 0, 0], [0, 0, 0, 0], [1706559510, 1706559540, 30.0, 3], [0, 0, 0, 0], [0, 0, 0, 0]]

Schedules === Before [[0, 0, 0, 0], [0, 0, 0, 0], [0, 0, 0, 0], [0, 0, 0, 0], [0, 0, 0, 0], [1706559600, 1706559630, 30, 3], [0, 0, 0, 0], [0, 0, 0, 0]]
Reply
#2
1 and 3 should be easy to fix.
I think I can find a solution for 2.

I will let you know as soon as I have an update.

I am also working on the results of your run_once tests.
<p><br></p>
Reply
#3
I think 1 and 3 are fixed.

For 2, I added a new global var gv.rn which indicates if a running program was started by Run Now.
It is 1 when a program was started by Run Now and 0 otherwise.

These are still in the run-once branch for now.
<p><br></p>
Reply
#4
(2024 Feb 03, 03:43 PM)dan Wrote: These are still in the run-once branch for now.

Perfect

(2024 Feb 03, 03:43 PM)dan Wrote: For 2, I added a new global var gv.rn which indicates if a running program was started by Run Now.

So upon receiving the signal "stations_scheduled" this will be set for the program that triggered the schedule. If this is correct then it works as expected


(2024 Feb 03, 03:43 PM)dan Wrote: I think 1 and 3 are fixed.


Found several issues

1) Signal "stations_scheduled" does not work any more in that the plugins are never triggered with this signal any more.  Signal changed to  "station_scheduled (i.e. station signular).  Intention or typo? I guess intentional because it signals one station at a time, but breaking change?
Code:
station_scheduled = signal("station_scheduled")


2) Menu PROGRAMS -> button RUN ONCE  throws an error 
Code:
File "/home/vagrant/Documents/sip/SIP/helpers.py", line 638, in run_program
    schedule_stations(p["station_mask"])
  File "/home/vagrant/Documents/sip/SIP/helpers.py", line 511, in schedule_stations
    report_station_scheduled()
TypeError: report_station_scheduled() missing 1 required positional argument: 'station'

 Fix
Code:
      report_station_scheduled()    ==>  report_station_scheduled(sid + 1)


3) Menu PLUGINS -> MANAGE PLUGINS -> button UPDATE ENABLED throws an error
Code:
xception in thread Thread-14 (restart):
::ffff:127.0.0.1:41722 - - [05/Feb/2024 13:24:22] "HTTP/1.1 GET /restart" - 200 OK
Traceback (most recent call last):
  File "/usr/lib/python3.10/threading.py", line 1016, in _bootstrap_inner
    self.run()
  File "/usr/lib/python3.10/threading.py", line 953, in run
    self._target(*self._args, **self._kwargs)
  File "/home/vagrant/Documents/sip/SIP/helpers.py", line 171, in restart
    GPIO.cleanup()
NameError: name 'GPIO' is not defined
Reply
#5
Yes, 1 changed to singular was intentional.
The old "stations_scheduled" seemed useless by returning "SIP".

I restored the old version for now along with the new version but the difference in names is too subtle.
Perhaps the new on should be named "one_station_scheduled"?

I added the fix for 2 and added an error check for 3.

When things are working properly for you I will merge the run-once branch into master.
<p><br></p>
Reply
#6
(2024 Feb 05, 07:28 PM)dan Wrote: Yes, 1 changed to singular was intentional.
The old "stations_scheduled" seemed useless by returning "SIP".

I restored the old version for now along with the new version but the difference in names is too subtle.
Perhaps the new on should be named "one_station_scheduled"?

I would leave it at "station_scheduled" as to me this sums the signal up correctly. Once I worked out what had changed the actual code change was trivial. The old name is used by 5 official plugins so that would be an easy change too. Going forwards I would keep the old signal implementation in parallel to the new one but document it as deprecated and remove it in a future release - standard software engineering practice. That would give any unofficial plugin writers time to adapt.

Using your plugin concept I did not find it too hard to achieve my goal for the plugin and I'm more or less happy with the current state so I have created a draft pull request.

I did have one more optional item on my todo list which was to respect the  Menu -> Settings -> Stations section -> Ignore Plugin adjustments? and to do this I would need to check gv.iw with something like:

        board byte = station_index / gv.nst
        station bit =   station index % gv.nst

but as mentioned in my post https://nosack.com/sipforum/thread-320-p...ml#pid1658 I'm not convinced about the board/station logic.
Reply
#7
The name "board" refers to a group of 8 stations.
   
It was originally intended for use with 8 channel shift registers and extension boards of that type.
<p><br></p>
Reply
#8
(2024 Feb 06, 04:14 PM)dan Wrote: The name "board" refers to a group of 8 stations.
It was originally intended for use with 8 channel shift registers and extension boards of that type.

I think that is something for the next version once I have played around with it bit.

I have tested the plugin again and think it is ready to be merged so I have change the pull request from draft to ready.
Reply
#9
I fixed the missing log entries for Run-once concurrent mode. 
I can't think of a good reason to log stations that have not run in sequential mode and prefer to keep the log as simple as possible.

Merged the Run-once branch into master.

Merged your pull request.

Problem:
The plugin could not be installed with the plugin manager.
Looking into it.
<p><br></p>
Reply
#10
The problem was a typo in README.md.

The plugin was listed as "monthly_adj_per_station".
Changed to " monthly_adjust_per_station". Installation works now.
<p><br></p>
Reply


Forum Jump:


Users browsing this thread: 2 Guest(s)