Skip to content

Plugin support #247

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

Closed
zcsahok opened this issue Feb 15, 2021 · 23 comments
Closed

Plugin support #247

zcsahok opened this issue Feb 15, 2021 · 23 comments

Comments

@zcsahok
Copy link
Member

zcsahok commented Feb 15, 2021

(just fyi, no action is needed)

I'm working on a proof of concept implementation of using contest-specific plugins. As a test I have defined the multi rules for MWC (https://memorial-ok1wc.cz/index.php?page=rules2l) that takes place each Monday from 1630 UT. For multiplier they use the last character of the worked call. The rationale behind this is likely to allow higher mult counts even if only a handful of DXCC entities participate.
Such plugins would allow Tlf to support practically any contest and would also be a solution for #235.

Here is the screenshot after today's contest, I still had some unworked mults on 80 and even more on 40.

image

As plugin technology I use Python as it is quite widespread and has a lot of features. The plugin script is implicitly selected by the contest rule config: for RULES=xdx the file rules/xdx.py is loaded if available.

Currently the interface methods are:

  • setup: called to reset the internal state
  • add_qso: called when a QSO is logged (either interactively or by reading the log)
  • nr_of_mults: number of mults per band or total, used for the display
  • is_multi: to tell if a call would be a multi, the functionality of bm_ismulti()

There are still a number of rough edges and some stuff are not too tlf-ish (i.e. bypassing multis[]), so work is in progress.

The code for MWC:

import tlf

mults = set()

def setup():
    global mults
    mults = set()

def add_qso(qso):
    multi_key = get_multi_key(qso.call, qso.band)
    if multi_key in mults:
        return
    mults.add(multi_key)

def nr_of_mults(band):
    if band == tlf.BAND_ANY:
        return max(len(mults), 1)
   
    bk = f'{band};'
    n = 0
    for multi_key in mults:
        if multi_key.startswith(bk):
            n = n + 1
    return n

def is_multi(qso):
    multi_key = get_multi_key(qso.call, qso.band)
    return not multi_key in mults


#######################################
#
# internal functions
#

def get_multi_key(call, band):
    a = call.split('/')
    base = a[0]
    if len(a) >= 2 and len(a[1]) > len(a[0]):
        base = a[1]
    return f'{band};{base[-1]}'
@airween
Copy link
Member

airween commented Feb 15, 2021

Hi Zoli,

do you already work on it?

This was my idea too exactly :).

If you have any working part, could you share this with us? We should make a new branch here (eg: dev/plugins).

@zcsahok
Copy link
Member Author

zcsahok commented Feb 15, 2021

commited the current state into https://github.com/zcsahok/tlf/tree/poc_plugin
it does compile, just don't run the tests.
plugin name is hardcoded, it looks for test.py
needs libpython3-dev
(the above screenshot was done with the plugin in action)

@dl1jbe
Copy link
Member

dl1jbe commented Feb 16, 2021

Great. Really good that you found time to start with it, The idea is flying around for quite some time.

it does compile, just don't run the tests.
Not here. Seems the linker flags are missing.

@dl1jbe
Copy link
Member

dl1jbe commented Feb 16, 2021

Found the reason for my compile problem. From the python3.8 "Whats New":

"To embed Python into an application, a new --embed option must be passed to python3-config --libs --embed to get -lpython3.8 (link the application to libpython). To support both 3.8 and older, try python3-config --libs --embed first and fallback to python3-config --libs (without --embed) if the previous command fails."

Adding --embed helped. In the long run that part needs to be moved to the configure.ac file.

@zcsahok
Copy link
Member Author

zcsahok commented Feb 16, 2021

Good to hear that you managed the compilation. I'm using stock 3.7 from Debian 10.
I was focusing on getting it functional, autoconf is one of the rough edges.

@dl1jbe
Copy link
Member

dl1jbe commented Feb 16, 2021

That needs to be sorted out, but not for the actual proof of concept. I will try to look for some other program with embedded python. Maybe I find one which uses autoconf

@airween
Copy link
Member

airween commented Feb 16, 2021

I've checked that repository, few remarks:

  • may be I forgot to set up something, but CTRL+C doesn't work; it's very annoying :)
  • I didn't have score/band window at top-right corner, therefore I can't check the state of suffixes/multipliers
  • I know the good and old Python saying: explicit is better than implicit, but when I started to play with the idea of this subsystem, I figured out that we should avoid to use the import tlf. Then I made a small test code, which is available here. I reviewed this code again, and realized then I made it for Python2, but there is a comment:
    import would be skipped, if you add your function to your builtin module. In Python3 (3.5), there is the PyModule_AddFunctions() function, but in the previous versions, you can make it like this snippet. - so there is a solution to avoid the import, if we want to follow that way (I see that you're using a constant, but as I remember there is an equivalent function in Python API to export the variables too, not just functions)

Now that's it after a quick review :).

Edit: for the record: I'm using Python3.6

@zcsahok
Copy link
Member Author

zcsahok commented Feb 16, 2021

Ctrl-C was disabled on purpose. The "official" reason is to let tlf_cleanup() always do its job and finally get a nice Thanks for using TLF.. 73 message. And it's quite easy to hit Ctrl-C unintentionally what is pretty annoying in the middle of a contest as it doesn't ask for confirmation. There are still 4 ways left to get out of Tlf: Alt-X, Alt-Q, :EXIt and :QUIt.

The plain answer is that Python throws an exception upon Ctrl-C and then this should be always handled complicating our life. So it's better to disable it altogether as we can live without it. (I also normally used Ctrl-C to exit...)

Re score window: I was using a plain contest and had the window. Don't know how can it get lost.

Importing: I didn't check adding to builtin. I was quite happy that it works at all, so did no further research. IMO having to write tlf.func() or tlf.var is not a problem.

@airween
Copy link
Member

airween commented Feb 16, 2021

Thanks for clarification, CTRL-C is clear now.

Missing of score window needs more investigation. Could you share the used logcfg.dat? May be some config directive is missing... (I've set up the CONTEST_MODE, CALL and RULE).

Importing: sure, that was just an idea. Btw tlf.func() would keep, only the import won't necessary.

@dl1jbe and also to you: here is a small patch which can helps to integrate the plugin:

https://github.com/zcsahok/tlf/compare/poc_plugin...airween:poc_plugin_ha2os?expand=1

As you can see I added the --enable-python-plugin config option. Pick up that patch (or let me know) if you like that and I can make a PR to your repository. We can add the other macros to check the feature is required (eg. HAVE_PYTHON_PLUGIN and skip the plugin if not...)

@dl1jbe
Copy link
Member

dl1jbe commented Feb 17, 2021

Wrt leaving the program: I think we should leave only ALT+X and :EXit for it. There may be some people using ALT+Q or :QUit but they will get used to it.

@airween

  • I think you miss the SCOREWINDOW keyword.

  • Just had a look at your patch for the configure.ac file. Looks good so far. It is a good start and we can make changes later if needed.

@zcsahok: I would suggest to not use RULES=xyz to load a python script implicitly. How about having a normal rules file and using a CONFIG_SCRIPT=<path_to_python_script> or similar there?

@airween
Copy link
Member

airween commented Feb 17, 2021

@dl1jbe: thanks, but no, SCOREWINDOW is in logcfg.dat - I just copied the default file to contest directory, it contains this keyword.

It's good to see that patch is well - I think it needs a minimal modification to make it usable (as I wrote it needs to handle the HAVE_PYTHON_PLUGIN or similar for the conditional macro).

More suggestions:

  • I also prefer the another keyword for plugin using, eg. PLUGIN=py:<path_to_script>. Using of RULES we lost the config directives like F1, ... which usually are in rules file
  • using the py: prefix gives a chance to make more plugin in the future (eg lua:)
  • I saw there is a plugin.c file - may be it's not too late to organize the structure, and make a plugins directory under the src, and put this file with name pythonplugin.c or similar
  • to avoid the sending of PR's between personal repositories, we should make a new branch in Tlf/tlf, eg. dev/plugin, and we can continue the collaborative work at there - ideas?

@ha5se
Copy link
Contributor

ha5se commented Feb 17, 2021

Two more little points:

  • I would suggest to leave out the py: prefix and the .py suffix, so that if later required, it would be possible to smoothly convert to other language without having to adjust the rules files. The shee-bang should suffice.
  • I would suggest a more descriptive keyword, e.g. PLUGIN_IS_MULTIPLIER= or the like, because other types of plugins may turn out to be necessary. For example, "RAEM" could be a good candidate for another type of plugin, to calculate the QSO-points.

@N0NB
Copy link
Member

N0NB commented Feb 17, 2021 via email

@dl1jbe
Copy link
Member

dl1jbe commented Feb 17, 2021

@dl1jbe: thanks, but no, SCOREWINDOW is in logcfg.dat - I just copied the default file to contest directory, it contains this keyword.

Than try Alt+R - it works here.

* to avoid the sending of PR's between personal repositories, we should make a new branch in `Tlf/tlf`, eg. `dev/plugin`, and we can continue the collaborative work at there - ideas?

+1

@airween
Copy link
Member

airween commented Feb 17, 2021

Than try Alt+R - it works here.

Thanks, it solved. But I still don't know why isn't there by default.

Btw counting multis and scoring works as well.

@airween
Copy link
Member

airween commented Feb 17, 2021

  • I would suggest to leave out the py: prefix and the .py suffix, so that if later required, it would be possible to
    smoothly convert to other language without having to adjust the rules files. The shee-bang should suffice.

if we leave the prefix or suffix and there won't be any explicit mark for the plugin type, we cut off the opportunity of further expansion (we must to assume that every plugin written in Python), so imho any explicit mark will help (and need).

Note: shebang works only if you run the script through a shell. Any embedded interpreter (Python, Lua) ignores it.

  • I would suggest a more descriptive keyword, e.g. PLUGIN_IS_MULTIPLIER= or the like, because other types of
    plugins may turn out to be necessary. For example, "RAEM" could be a good candidate for another type of plugin, to
    calculate the QSO-points.

I think the PLUGIN_IS_MULTIPLIER describes just partially the advantage of plugins. Eg. we should (so, we must) to use the plugin scripts for scoring too. IMHO the PLUGIN=py:test or PLUGIN=test.py will be fine. But just an opinion...

@zcsahok
Copy link
Member Author

zcsahok commented Feb 17, 2021

The idea of the plugin is that we have a collection of optional methods that it can be implement per contest. The ones where the core Tlf functionality is fine one can simply leave out.

Currently the most critical feature is the handling of multis, that's why it's in the focus. For RAEM of course the quite complex score() method has to be implemented instead.

PLUGIN=test.py (or CONTEST_PLUGIN) is fine as the file is named anyway like that and the extension indicates the technology. (but IMO we shall stick with one technology or it'll be a mess)

@zcsahok
Copy link
Member Author

zcsahok commented Feb 17, 2021

Re: exit: as it's independent of the plugin story I'll prepare a PR for the master. How about removing Alt-Q but keeping :QUIt? So we gain a free key combination and for the command entry it makes no difference. (Alt-X is quite handy as the key are very close)

@dl1jbe
Copy link
Member

dl1jbe commented Feb 17, 2021

Re: exit: as it's independent of the plugin story I'll prepare a PR for the master. How about removing Alt-Q but keeping :QUIt? So we gain a free key combination

Fine by me. If possible please add the other changes (ctrl-C, mode to static , ...) to the PR.

and for the command entry it makes no difference. (Alt-X is quite handy as the key are very close)

Right, and that is what I use always.

@ha5se
Copy link
Contributor

ha5se commented Feb 18, 2021

  * I would suggest a more descriptive keyword, e.g.  |PLUGIN_IS_MULTIPLIER=| or the like, because other types of
    plugins may turn out to be necessary. For example, "RAEM" could be a good candidate for another type of plugin, to
    calculate the QSO-points.

I think the PLUGIN_IS_MULTIPLIER describes just partially the advantage of plugins. Eg. we should (so, we must) to use the plugin scripts for scoring too. IMHO the PLUGIN=py:test or PLUGIN=test.py will be fine. But just an opinion...

Well, my suggestion above intended just to prepare for a possible future separate second (scoring) plugin. To elaborate, I think of something like PLUGIN_IS_MULTIPLIER= , PLUGIN_IS_MULTIPLIER2= and PLUGIN_QSOPOINT= .
(Perhaps new plugin types may also be introduced later...)

  • The second multiplier rule above could be used for contests with more than one type of multipliers. For example,
    HQ-id vs. ITU zone for IARU HF, or HA-county vs. DXCC for HA-DX.
  • All plugins could be optional, then using built-in code utilizing existing rule keywords. This would provide backward compatibility
    and also smooth gradual developing and testing the plugin support, while others still can use the old code and old rules files.
  • A non-programmer end-user could easily create a new contest config for himself simply selecting from existing plugins --
    similarly simple as so far.
  • Code duplication could also be easier avoided for cases with same multipliers but different scoring, or the other way round.
  • It would also be possible to specify plugins in different programming languages within a single contest.
  • Last but not least, plugins could be kept simpler than overloading more functions into a single module.

Admitting, I only see advantages for this. Just another opinion, of course... IMHO, during proof-of-concepts phase, brain storming can help much more to build final concept as well as to avoid some unnecessary coding efforts, than any other discussions during a later phase...

@airween
Copy link
Member

airween commented Feb 18, 2021

Well, my suggestion above intended just to prepare for a possible future separate second (scoring) plugin. To elaborate, I think of something like PLUGIN_IS_MULTIPLIER= , PLUGIN_IS_MULTIPLIER2= and PLUGIN_QSOPOINT= .
(Perhaps new plugin types may also be introduced later...)

I think this is a bit complicated. I believe in one contest - one plugin rule file.

Instead of this, it would be better to implement the is_multi2(), is_multi3(), ... in the rule script (and inside of Tlf), and if that isn't used, then just simply return False.

The another solution would be if the is_multi() returns a complex type, eg. a dict or list, but this limits the another implementations (I mean if the other plugin doesn't have that type...).

@zcsahok
Copy link
Member Author

zcsahok commented Mar 1, 2021

Created dev_plugin branch. It contains just plugin.c/h and minimal changes to main+readcalls, plus the automake machinery by @airween.

The actual plugin points and functions we will figure out later. I'll keep my branch for prototyping and merge things into dev_plugin as needed.

@zcsahok
Copy link
Member Author

zcsahok commented Mar 20, 2023

Covered by #347, closing issue.

@zcsahok zcsahok closed this as completed Mar 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants