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

Version 1.0.0 #163

Merged
merged 55 commits into from
Feb 23, 2020
Merged

Version 1.0.0 #163

merged 55 commits into from
Feb 23, 2020

Conversation

Shaquu
Copy link
Member

@Shaquu Shaquu commented Oct 15, 2019

Fixed

  • Node id macify algorithm changed
  • Corrections regarding issue #12 so that changes can be deployed without restarting node-red
  • Automatically creating a new service and replacing the old one if the service type changed
  • Automatically replacing an accessory with a new one if the accessory information changes (e.g. Name, Manufacturer, ...)

Added

  • After Service selection in node configuration Category will be automatically set to default for Service
  • Interface Name for Camera Service configuration
  • Support for new TV Remote services

Changed

  • Accessory Category in node configuration moved under Service selection
  • Clarify NO_RESPONSE in README
  • Update node-red version in dependencies
  • Camera Service source code to match newest improvements in homebridge-camera-ffmpeg
  • Update to latest HAP-NodeJS
  • Removed unnecessary accessory category from service node
  • Removed fields Manufacturer, Serial Number and Model from linked service nodes

@Shaquu Shaquu added the help wanted Used for setup help label Oct 16, 2019
@Shaquu
Copy link
Member Author

Shaquu commented Oct 16, 2019

@NRCHKB/test we are close to releasing this version.
Please test dev branch.

Past showed that we have to make real tests:

  • creating new flows in node-red from scratch
  • running old flows to check if they are compatible
  • integrations
  • and more

@Shaquu
Copy link
Member Author

Shaquu commented Jan 28, 2020

I need to look at #185 and you promised to look at #194 :)
And some minor bugs.
Plus I am not sure if current Service getOrCreate is correct - I have to check if finding is working and not overwriting anything blindly.

@radokristof
Copy link
Contributor

Yes I'm working on it... I'm a little bit late with my hobbies... The reason I didn't moved forward because I either have to try or have a deeper look at ffmpeg doc that which options are not necessary and only optional. Also I want to have this to be consistent in the Camera node, so I also had to this for Audio...

- send message two both outputs on onIdentify
- updated mocha and eslint-config-prettier dependencies
@Shaquu
Copy link
Member Author

Shaquu commented Jan 28, 2020

No presure. If I understand better what you wanted to fix maybe I can take that task.

@radokristof
Copy link
Contributor

radokristof commented Jan 28, 2020

I'm happy if I can help...
Basically what is needed that the additional video and audio args should not be a required option. Right now if you clear one of the options like 'Video Filter', it will show an error in Node-RED, but the bigger problem is that it will not leave that empty, but pass the default parameter to ffmpeg.
For example for Video Filter: scale=1280:720
But if you want to use copy as video codec, which should be the most efficient, you can't use it right now, because it will pass the scale=1280:720 as an option and you can't use copy and scale together, ffmpeg will exit with 1.
So I think, which parameters are not required by ffmpeg default that should be also optional in Node-RED (I might think that all of the additional args are optional).
So basically what is needed right now, I think, that remove the parts from the code where these parameters are initialized with the default parameter and only pass that option if it is specified by the user. Like it is how done with video filter here:

(vf.length > 0 ? ' -vf ' + vf.join(',') : '') +

Why this is not working right now as I want it, because earlier, it is initiliazed like this:
? 'scale=' + width + ':' + height + ''

Something similar could be done with each parameter. So you can fully customize the command ran by ffmpeg through Node-RED.

Hope this made it clear for you :) So this might not be really related to the issue you linked above.

-   Video Filter value in Camera Control is now optional [#194](#194) (can be empty, before it was generated if was empty)
-   Removed updateReachability as it is deprecated (and doesn't make a difference)
@crxporter
Copy link
Member

How are we doing with this? Are getting to a point we can let it into the wild with plans to get a 1.0.x bug fix fo issues that come up?

I think we will see a lot more testers (and possibly bug reports) show up once it’s available in the palette manager.

@sjorge
Copy link

sjorge commented Feb 23, 2020

Yeah a 1.0.0 release with a .1 bug fix release if a lot of bugs are found sound good to me

@Shaquu
Copy link
Member Author

Shaquu commented Feb 23, 2020

Well let me look at situation and I might make a release in few hours.

@radokristof
Copy link
Contributor

@Shaquu Thanks!
Just a short reminder: If upgrading from 0.8, you will have to remove the Bridge and re-add it.
This will be enough? Or you need to make some extra steps? (Like configuring new bridge or anything else).

@Shaquu
Copy link
Member Author

Shaquu commented Feb 23, 2020

@radokristof my friend, you should copy your production env, install dev and tell me :)

@radokristof
Copy link
Contributor

I have tested this version multiple times on my test system, which was upgraded from 0.8 once, and if I remember correctly that was enough (maybe I did another node-red restart just to make sure).
I just wanted to warn everyone and see if anything changed in terms of this :)

@Shaquu
Copy link
Member Author

Shaquu commented Feb 23, 2020

Important notice regarding release 1.0.0:

  1. This release brakes backward compatibility. In order to make everything work you will have to remove all NRCHKB bridges from Home.app and add them again.
  2. node-red nodes output order changes, first is for onChange event, second for onSet and third for camera snapshots (this was second output previously).
  3. cameraSnapshot output (third node output) is only visible on Camera Service node

@crxporter
Copy link
Member

Important notice regarding release 1.0.0:

Can these notes be added as a warning in palette manager when a user is upgrading?

Many nodes have “this update will require node red restart” ... is there a custom warning option?

@Shaquu Shaquu merged commit a97b126 into master Feb 23, 2020
@radokristof
Copy link
Contributor

Publish failed...

@Shaquu
Copy link
Member Author

Shaquu commented Feb 23, 2020

@crxporter this train is gone :)
@radokristof don't worry! published manually

@radokristof
Copy link
Contributor

radokristof commented Feb 23, 2020

@Shaquu updated my production to 1.0.1.
Everything went as I said, just needed to remove bridges and re-add them.
However, Camera Accessories were shown as unsupported, I had to remove them, deploy and re-add a new camera node. After this, it worked as it should.

The video filter is now optional on Cameras, that works, but Node-RED GUI shows an error, because it thinks that the video filter is required...
Also we need to think about what other parameters should be optional and not required/generated automatically. I think most of them can be optional...

@jonathanschneider
Copy link
Contributor

I installed 1.0.1 but my nodes still only have two outputs. What am I missing?

@crxporter
Copy link
Member

@jonathanschneider - two outputs is correct for all but camera service I believe. If your camera service items still have just the two outputs then something may have gone wrong?

@radokristof
Copy link
Contributor

I my all nodes also only has two outputs. However these two are all onChange and onSet outputs. Seem that the snapshot output is gone.
However after testing it for a half day in my production system:

  • It feels much faster, now I don't have incorrect states when launching the app (previously I had to wait to load the states and also sometimes had to re-enter the app).
  • When accessing remotely (ie.: through a hub) it works instantly, whereas before sometimes I got No_Response states and had to restart the app several times to connect...
  • Automations also seem to work more reliable than before.

@jonathanschneider
Copy link
Contributor

jonathanschneider commented Feb 24, 2020 via email

@Shaquu
Copy link
Member Author

Shaquu commented Feb 24, 2020

@jonathanschneider my mistake:
cameraSnapshot output (third node output) is only visible on Camera Service node

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Used for setup help
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants