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

multicastAlarm added as function #307

Merged
merged 53 commits into from
Dec 19, 2017

Conversation

f-kessler
Copy link
Contributor

includes/decoders/poc.py extended to include includes/expressAlarm.py
added options to the config template and testdata for testing

Express-Alarm function added
added options for Express-Alarm
added options for Express-Alarm
new function to support Express-Alarm
added processing of Express-Alarms
added test data for Express-Alarm
@f-kessler f-kessler changed the title Express-Alarm als Funktion hinzugefügt Express-Alarm added as function Sep 19, 2017
@@ -83,8 +83,24 @@ POCSAG512: Address: 3333333 Function: 4 Alpha: BOSWatch-Test: invalid
POCSAG1200: Address: 7777777 Function: 1 Alpha: BOSWatch-Test: denied

# out of filter Range
POCSAG1200: Address: 0000004 Function: 1 Alpha: BOSWatch-Test: out of filter start
POCSAG1200: Address: 9000000 Function: 1 Alpha: BOSWatch-Test: out of filter end
#POCSAG1200: Address: 0000004 Function: 1 Alpha: BOSWatch-Test: out of filter start
Copy link
Owner

Choose a reason for hiding this comment

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

wieso sind die beiden Zeilen 86 und 87 auskommentiert? versehen oder Absicht?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Versehen...

# Using Express-Alarm (0 - off | 1 - on)
expressAlarm = 0

# time limit for alarms that do not belong to the Express-Alarm sequence
Copy link
Owner

Choose a reason for hiding this comment

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

time limit - in Minuten? Sekunden? Bitte noch vermerken...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Da hast du natürlich Recht. Was bringt die Option ohne die Einheit zu wissen.

expressAlarm_ignore_time = 15

# Express-Alarm delimiter RIC
expressAlarm_delimiter_ric =
Copy link
Owner

@Schrolli91 Schrolli91 Sep 19, 2017

Choose a reason for hiding this comment

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

was ist die delimiter_ric? bzw für was wird die verwendet? evtl hier das Kommentar noch etwas präziser ausführen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wird direkt vor der Express-Alarm Sequenz gesendet. Es soll aber auch Netze geben die das so nicht machen. Ich nutze sie um die expressList zu leeren. Werde das mal ergänzen.

Copy link
Owner

Choose a reason for hiding this comment

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

Alles klar... Notfalls schreibe einen kleinen Roman :D aber eine Option zu haben, bei der die Funktion nicht klar ist, bringt am Ende ja niemanden etwas 👍

@Schrolli91
Copy link
Owner

Schrolli91 commented Sep 19, 2017

@f-kessler ein paar Dinge habe ich ja bereits als direkte Kommentare im Code angemerkt. Einfach nochmal anschauen, oder dazu antworten.
Aber bitte auf jeden Fall anschließend noch die CHANGELOG.md anpassen, bzw das neue Feature dort eintragen :)

Ansonsten sieht das ganze aber recht gut aus, was meint Ihr? @flothi @thejockel @PeterLaemmle

expressAlarm_ignore_time = 15

# Express-Alarm delimiter RIC
expressAlarm_delimiter_ric =
Copy link
Owner

Choose a reason for hiding this comment

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

todo Genaue Funktion der delimiter_ric dokumentieren

# -*- coding: utf-8 -*-

"""
expressAlarm is the function to enable BOSwatch to deal with Swissfone Express-Alarm
Copy link
Owner

Choose a reason for hiding this comment

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

Gibt es zum Swissfone Express-Alarm irgend ein offizielles Dokument, welches man als Referenz evtl direkt im Kommentar verlinken könnte?

Copy link
Contributor Author

@f-kessler f-kessler Sep 21, 2017

Choose a reason for hiding this comment

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

Mein Wissen darüber ist aus vielen Beträgen in den einschlägigen Foren zusammengesucht. Ich kann mal schauen ob ich da noch etwas zusammenhängendes finde. Swissphone hält die Infos dazu leider ziemlich unter Verschluss.

Copy link
Owner

Choose a reason for hiding this comment

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

Eine prinzipille Beschreibung wie EA abläuft wäre aber auf jeden Fall nicht verkehrt, damit sich jemand der es nicht kennt was drunter vorstellen kann ;-)
Kann mir auch vorstellen, dass es viele in Ihren Funkkreisen haben aber nichts davon, bzw dessen Funktion wissen.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ich habe mir mal eine Beschreibung einfallen lassen. Außerdem habe ich mir überlegt, dass es vermutlich besser wäre, wenn ich den von Swissphone geschützten Markennamen Express-Alarm aus dem Code entferne. Da sich damit sehr viel geändert hat, bin ich aber unsicher, ob ich den neuen Code einfach hinzufügen, oder ob ich einen neuen branch und pull request machen soll.
Was ist denn das sinnvollste vorgehen?

Copy link
Owner

Choose a reason for hiding this comment

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

hinzufügen... sollte es ein problem geben, kann man ja ohne Aufwand zum jetzigen Stand reverten ;-)

@Schrolli91
Copy link
Owner

Schrolli91 commented Oct 3, 2017

edit debug msg
die POCSAG Daten werden im decoder sowieso einmal ausgegeben. Daher reichen in den Funktionen dann nur kleine Infos was gerade passiert.

@Schrolli91
Copy link
Owner

del typ and freq in adding routine
In der newEntry Rountine werden typ und freq sowieso nicht verarbeitet.
Nur beim Ausführen des Alarms werden dann von der msg RIC die Daten übernommen wobei typ auch hier immer fest ist, da POCSAG

@Schrolli91
Copy link
Owner

Schrolli91 commented Oct 3, 2017

insgesamt würde ich aber auch versuchen wollen, die gesamte Funktion möglichst nahe an den Ablauf des doubleFilters anzulegen. Damit nicht 2 verschiedene gedankliche Konzepte vorhanden sind, sondern alles der selben Logik folgt, wenn du verstehst was ich meine?

Bsp:
for (xID, xTimestamp, xMsg) in doubleList:
Einfach besser lesbar durch die direkte Benennung der Daten

@f-kessler
Den Umbau würde ich auch machen, wenn das Ok wäre?
Müsste danach halt nochmal getestet werden!

@f-kessler
Copy link
Contributor Author

Da wird mir mein Anfängerwissen in Bezug auf Python und Programmierung im allgemeinen wieder zum Verhängnis. Ich fand die Variante mit for (xID, xTimestamp, xMsg) in doubleList: schwieriger zu lesen als for i, _ in enumerate(multiList): Schaue ich mir nochmal an.

Deine Änderungen bezüglich debugging finde ich absolut schlüssig. Das debugging habe ich bisher auch eher stiefmütterlich behandelt.

In der Funktion multicastAlarmExec müssen nach deinen Änderungen noch die Indexe angepasst werden. Habe das gerade nachgeholt.

Vielen Dank für die Unterstützung!

@Schrolli91
Copy link
Owner

Gerne... Wenn du magst würde ich das mit dem for xxx in xxx: umsetzen und einbauen

@Schrolli91
Copy link
Owner

@f-kessler teste das bitte mal und Poste bei einem evtl auftretendem Fehler bitte den Logauszug

@f-kessler
Copy link
Contributor Author

multicastAlarm und die Descriptions funktionieren. Danke für die Anpassung.

@Schrolli91
Copy link
Owner

Ich schau im laufe des Abends nochmal, ob ich Kleinigkeiten finde...
Und dann stoßen wir nochmal einen Offiziellen Test an, bevor wir mergen ;-)

Ich hab zu danken, die meiste Arbeit kam ja immerhin von dir

@f-kessler
Copy link
Contributor Author

Habe gerade nochmal getestet. Ich bekomme jetzt folgenden Fehler:
Traceback (most recent call last): File "/home/user/BOSWatch/includes/decoders/poc.py", line 138, in decode multicastAlarm.newEntrymultiList(data) File "/home/user/BOSWatch/includes/multicastAlarm.py", line 34, in newEntrymultiList multiList.append([data, timestamp]) UnboundLocalError: local variable 'multiList' referenced before assignment

Das hängt wohl mit der Änderung von Zeile 25 in 63f2e58 zusammen. Wenn ich die Zeile wieder hinzufüge, funktioniert es ohne Fehler.

@Schrolli91
Copy link
Owner

@f-kessler Hopper'la - Da hast du natürlich absolut Recht, habe das eben gefixt.
Jetzt dürfte der Laden aber laufen ;-)

Warten wir noch ein bis zwei tests von anderen Usern ab - Dann würde ich sagen merge ich
Nochmals Danke für die hervorragende Arbeit

@Schrolli91 Schrolli91 self-assigned this Oct 4, 2017
If not excluded it cannot be used anywhere
Copy link
Contributor Author

@f-kessler f-kessler left a comment

Choose a reason for hiding this comment

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

@Schrolli91 Schrolli91 merged commit 4811477 into Schrolli91:develop Dec 19, 2017
@ghost ghost removed the Review needed label Dec 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants