PR consolidating multiple fixes as provided by users across 3 "Issue" threads #102
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
As provided by star-glider in issue: nutechsoftware#99 The custom notification option is broken; the notify_data dictionary is always empty. There's quite a bit of code that works over this dictionary (lines 1182 through 1204 in types.py), but none of it runs because custom_values is blank. I haven't dug in deeply enough to really understand what's going on here; it's way more code than exists in the other notification methods, and I'm not sure why. I ran into this trying to set up Slack notifications (JSON POST) and threw in a quick patch to get it working. Basically, I just skip over all of the inactive code, create a dictionary directly in a manner similar to the other notification methods, and then reference that in the _do_post method call. Since the project isn't being maintained, I don't think it's worth the time to submit a pull request (and do the associated work to figure out why there's all that additional code), but I figured I'd drop my patch in here just in case it helps anyone. Two notes: 1) the modification to line 1144 was just me troubleshooting; it's not relevant to the fix but is kind of a nice-to-have; 2) the new "message_string" variable includes a version of the "Subject" option from the email notification, but I just harcoded in the message instead of modifying the web forms to offer it as a front-end-selectable option, mainly because I'm lazy. Presumably, if you're applying this patch, you can just type in whatever you want there or just set it equal to the 'text' variable to eliminate it entirely. I know the project is essentially abandoned, but it's a great addition to these old (and very reliable) ADEMCO systems. As long as you're running it behind a firewall with no ports exposed, there's really not much risk in running an old image with so little attack surface, and while old it works just perfectly (kind of like my 20-year-old alarm system)!
…t Loading From jimmyhchan nutechsoftware#93 I have manually setup an raspbian image on a raspberrypi 3 and have most of everything working with a customized version of the docker file. Unfortunately, the virtual keypad was not working. The symptoms is the same as outlined here https://www.alarmdecoder.com/forums/viewtopic.php?f=3&t=1167&p=4267&hilit=keypad+loading#p4267 Steps to repro: setup the webapp go through the setup flow navigate to the keypad page BUG: Keypad only shows Please Wait Loading... Clicks on any button causes the log ERROR: default_error_handler: method_access_denied, You do not have access to method "on_keypress" (endpoint=/alarmdecoder, msg_id=None) [in /usr/local/lib/python2.7/dist-packages/socketio/virtsocket.py:51] Root cause I debugged a bit and found the root cause is user_id is None in the recv_connect method which calls add_acl_method: https://github.com/nutechsoftware/alarmdecoder-webapp/blob/master/ad2web/decoder.py#L829 I could not find anything which sets the user_id for the session. Perhaps a dependency api changed. Fix/workaround I have a fix using user_is_authenticated(current_user) instead. Happy to create a real PR if someone can confirm this is the desired approach.
Changed 'user_id' to '_user_id' to support changes in Flask-login 0.5.0 - requires requirements.txt update to Flask-login == 0.5.0
Update the latest compatible version of most modules.
|
thanks, I will try to review this this weekend |
ajschmidt8
reviewed
Mar 9, 2025
| @@ -1175,6 +1175,8 @@ def _do_get(self, data, app=None): | |||
| @raise_with_stack | |||
| def send(self, type, text, raw): | |||
| self.msg_to_send = text | |||
| message_string = 'Primary Residence Alarm Event: %s' % text | |||
There was a problem hiding this comment.
The user who contributed this change in #99 stated that this string is hardcoded for his particular use case:
the new "message_string" variable includes a version of the "Subject" option from the email notification, but I just harcoded in the message instead of modifying the web forms to offer it as a front-end-selectable option, mainly because I'm lazy. Presumably, if you're applying this patch, you can just type in whatever you want there or just set it equal to the 'text' variable to eliminate it entirely.
Therefore, it doesn't make much sense to merge this change.
I would recommend either amending this change to get text from the web form or removing it from this PR altogether.
|
@ajschmidt8 let me know if you'd like me to delete and resubmit, or if it's possible to merge as is (and if required backout the single item). As mentioned, I'll have to redo the full PR as I only can use web based git to submit the PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@ajschmidt8
As requested, creating a PR which pulls together changes from:
#93
#99
and
#92
I'm not an expert here but I can say that I've been running these changes for months (and with the updates requirements.txt for weeks) without issues on my setup.
In short: