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

[xmppclient] Add send image throught HTTP #11247

Merged
merged 1 commit into from
Oct 19, 2021
Merged

[xmppclient] Add send image throught HTTP #11247

merged 1 commit into from
Oct 19, 2021

Conversation

gkfabs
Copy link
Contributor

@gkfabs gkfabs commented Sep 13, 2021

When I have an alert on my phone, I would like to be able to see an image which comes from the corresponding camera. For example when I feed the fishs or when I am not at home, the security camera corresponding to the PIR.

At first I wanted to integrate the jingle file transfert. But I think it will take time to see it in Smack igniterealtime/Smack#489

This is why I did a small improvement using https://github.com/igniterealtime/Smack/tree/master/smack-experimental/src/main/java/org/jivesoftware/smackx/httpfileupload

But I am getting some problems to add a new library: smack-experimental.

Thanks

@Skinah Skinah added the enhancement An enhancement or new feature for an existing add-on label Sep 14, 2021
Copy link
Member

@pavel-gololobov pavel-gololobov left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks.

@gkfabs
Copy link
Contributor Author

gkfabs commented Oct 7, 2021

oob has not been implemented yet in smack. It has been running ok on my local for now, so I would leave as is. Any suggestion?

@gkfabs gkfabs changed the title [xmppclient][WIP] Add send image throught HTTP [xmppclient] Add send image throught HTTP Oct 7, 2021
Copy link
Contributor

@lolodomo lolodomo left a comment

Choose a reason for hiding this comment

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

LGTM

I am just not enough expert in dependencies to understand if this is normal that a dependency is added in feature file but not in POM (bcprov).

@lolodomo
Copy link
Contributor

lolodomo commented Oct 9, 2021

I let another maintainer merge this PR as I am not sure about the dependencies stuff (bcprov).

@lolodomo lolodomo added the cre Coordinated Review Effort label Oct 9, 2021
@lolodomo
Copy link
Contributor

@wborn : can you have a look on this one?

@lolodomo
Copy link
Contributor

@fwolter : maybe you can have a quick look, at least to confirm that everything is as expected for the dependency?

@fwolter
Copy link
Member

fwolter commented Oct 17, 2021

You should be able to set the scope to provided in the pom.xml, if you add the dependency via features. Otherwise the dependency will be added to the binding's JAR and downloaded separately when installing the binding, AFAIK.

Copy link
Member

@fwolter fwolter left a comment

Choose a reason for hiding this comment

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

The dependency in general should be okay. I'm not aware of, that there's any provided method to upload files via HTTP.

Signed-off-by: Fabien Carrion <fabien@carrion.mx>
Copy link
Member

@fwolter fwolter left a comment

Choose a reason for hiding this comment

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

I think it's okay to add a transitive dependency to features.xml, but not to the pom.xml. @openhab/add-ons-maintainers please correct me if I'm wrong.

@gkfabs Can you file another PR, setting the dependencies in the pom.xml to provided?

@fwolter fwolter merged commit 8257179 into openhab:main Oct 19, 2021
@fwolter fwolter added this to the 3.2 milestone Oct 19, 2021
@gkfabs
Copy link
Contributor Author

gkfabs commented Oct 19, 2021

@fwolter Do you want me to set all the smack libs dependencies to provided or only the new one smack-experimental?

@fwolter
Copy link
Member

fwolter commented Oct 19, 2021

Each dependency, which exists in features.xml.

frederictobiasc pushed a commit to frederictobiasc/openhab-addons that referenced this pull request Oct 26, 2021
Signed-off-by: Fabien Carrion <fabien@carrion.mx>
dschoepel pushed a commit to dschoepel/openhab-addons that referenced this pull request Nov 9, 2021
Signed-off-by: Fabien Carrion <fabien@carrion.mx>
Signed-off-by: Dave J Schoepel <dave@theschoepels.com>
NickWaterton pushed a commit to NickWaterton/openhab-addons that referenced this pull request Dec 30, 2021
Signed-off-by: Fabien Carrion <fabien@carrion.mx>
Signed-off-by: Nick Waterton <n.waterton@outlook.com>
nemerdaud pushed a commit to nemerdaud/openhab-addons that referenced this pull request Jan 28, 2022
Signed-off-by: Fabien Carrion <fabien@carrion.mx>
marcfischerboschio pushed a commit to bosch-io/openhab-addons that referenced this pull request May 5, 2022
Signed-off-by: Fabien Carrion <fabien@carrion.mx>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cre Coordinated Review Effort enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants