nora-service
nora-service copied to clipboard
Locks
@andrei-tatar I saw you merged in the lock changes to the service. Has that been deployed? I'm getting an "Invalid object for sync" error and don't see any obvious disconnects in naming between the node and services files. I'll keep looking in the meantime.
Thanks
Yes, you can see the changes here: https://github.com/andrei-tatar/nora-service/pull/9
I also added isJammed
in the state, so the node-red plugin needs to provide it.
export interface LockUnlockState extends State {
isLocked: boolean;
isJammed: boolean;
}
Also, please don't open new issues if there's already one for that topic (https://github.com/andrei-tatar/nora-service/issues/7). Just reopen closed ones or add comments to them.
Thanks @andrei-tatar Will do going forward on reopening. I had originally put in the isJammed but wasn't sure the best way to do it in the node. Wasn't sure if should just make it a different payload for jammed/unjammed or set it up as a msg.payload.jammed (along the lines of what you did in the light node). Any thoughts on what would be a cleaner / more consistent approach with the other nodes?
@andrei-tatar I'm still trying to get comfortable with the node code base. I'm slowly thinking I'm understanding it (I haven't written real code in 20 years so am trying to understand it as I go), but somehow I've done something or have a typo I"m not seeing that is creating the following error
src/nodes/nora-lock.ts:53:49 - error TS2345: Argument of type 'LockState' is not assignable to parameter of type 'boolean'.
53 tap(([_, state]) => notifyState(state)),
~~~~~
Here's the file - https://github.com/rgerrans/node-red-contrib-nora/blob/master/src/nodes/nora-lock.ts
Any suggestions or guidance would be greatly appreciated (I'm just trying to get a base node to work then will play with the payload structure).
@rgerrans
notifyState
is the function used to take the state of the node and update the node status string.
You defined it as:
function notifyState(isLocked: boolean) {
stateString$.next(`(${isLocked ? 'locked' : 'unlocked'})`);
}
you could define it as:
function notifyState(state: LockState) {
stateString$.next(`(${state.isLocked ? 'locked' : 'unlocked'}:${state.isJammed?'jammed':'-'})`);
}
and just call it on the state object. Instead of:
const lvalue = state.isLocked;
const jvalue = state.isJammed;
notifyState(state.isLocked);
notifyState(state.isJammed);
you can just: notifyState(state);
Your code actually calls it with a state object but the function is defined to accept a boolean. That's why it fails.
Ahhhh, that makes sense. I wasn't looking all the way to the bottom to see that function and close the loop on the issue. Thanks!!!
Vs Code helps a lot to navigate through the code. Just ctrl + click :)
Thanks. It's more foundational than that, where I've never worked that much in javascript so don't always know when I'm looking at a defined function vs. a standard call ;-) I've been trying to step through everything to make sure I understand but didn't make it to the end to realize that was a function.
@andrei-tatar I think I have it working correctly on the Node-Red side receiving commands from Google but it's having an issue when I send commands to Google. I get a invalid object for update
error when I try to push a change to the node.
Edit: Deleted the reference to Execute since I realized that I was probably looking in the wrong file since the command from Google comes through fine. Any suggestions on where to look to try to troubleshoot? Thanks.
@rgerrans I think you need to send the whole state object. Not just isLocked, but also isJammed.
@andrei-tatar Thanks. I actually had a typo that I finally found.... Will test a bit before sending a pull request.
@rgerrans wouldn't be cleaner and easier to treat isJammed differently? I assume most of the people won't use it anyway. So have input/output payload only for isLocked. isJammed would be set when a message with topic "jammed" arrives or something similar.
@andrei-tatar I know, I was definitely going back and forth on taking a different approach to be able to keep locked in the main payload. I had toyed with the idea of doing it along the lines of what you did with lights and brightness. I hadn't thought about using the topic, how would you then treat the outbound msg from the node?
Also, Google didn't treat isJammed: true
like I thought it would. It still changed lock status on request even if isJammed: true
was set? I saw that there was an error code that should be thrown in that situation but couldn't quite understand how to use your error code pull to check that in order to block the lock/unlock action?
Edit: Realized I wouldn't need an outbound "isJammed" message from the Node. I'll go with the Topic approach for inbound messages. I'll also try to put in a blocker/error message if the isJammed: True
Probably take me a few days with the holidays to get back to this.
@andrei-tatar So I implemented the "Jammed" topic approach. I also put in error messages if they use Google and the lock reflects Jammed status. Google still responds with the requested action, but the node won't send any commands and sends an error message instead. Let me know any other thoughts on changes. Otherwise the pull should be ready to go
@andrei-tatar I realized that two of my Lock nodes never deployed to Google. I was just trying to recreate them to see if it was just something that happened during testing and ran into a weird issue. If I change the Unlock value from binary | false to number "0" it doesn't retain when I save the config change. But I can change it to string "0", deploy that and then change it to number "0". Not sure if the bug is on my end or on Node-Red's? It works fine for the Lock value if I go straight from binary|true to Number|255 or Number|0? I don't see any difference in the code between the two. Any thoughts on what I might be missing?
@rgerrans probably invalid state. If the state is invalid (does not pass schema check), new devices won't get synced.
@andrei-tatar That makes sense. When I deleted and recreated they created the Google end points.
Let me know any thoughts on that bug in setting the UnlockValue? Otherwise everything seems to be testing out fine.