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

[bondhome] New binding: Bond Home #7260

Closed
wants to merge 4 commits into from
Closed

Conversation

SRGDamia1
Copy link

New Binding for Bond Home bridge and devices

A new binding for RF and IR remote control devices controlled by a Bond Bridge. (https://bondhome.io/products/bond-bridge/)

@cpmeister cpmeister added the new binding If someone has started to work on a binding. For a new binding PR. label Mar 31, 2020
@TravisBuddy
Copy link

Travis tests were successful

Hey @SRGDamia1,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

@cpmeister cpmeister changed the title New binding: Bond Home [bondhome] New binding: Bond Home Mar 31, 2020
@TravisBuddy
Copy link

Travis tests were successful

Hey @SRGDamia1,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

2 similar comments
@TravisBuddy
Copy link

Travis tests were successful

Hey @SRGDamia1,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

@TravisBuddy
Copy link

Travis tests were successful

Hey @SRGDamia1,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

@digitaldan
Copy link
Contributor

Hi @SRGDamia1 , i had never heard of Bond Home, cool product.

I took a minute to look at their api, i notice you mention you needed to manually find your Bond's IP address, but according to their docs, they support standard mDNS, and openHAB makes that very easy to add to your binding for auto discovery.

http://docs-local.appbond.com/#section/Getting-Started/Finding-the-Bond-IP

you can find a ton of examples from other bindings:
https://github.com/openhab/openhab-addons/search?l=Java&q=mdns

Good Luck!

@SRGDamia1
Copy link
Author

@digitaldan - I'm pinging for the IP from the serial number, but using mDNS would probably be better to get the bridge into the inbox. I'll look into it.

@TravisBuddy
Copy link

Travis tests were successful

Hey @SRGDamia1,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

@SRGDamia1
Copy link
Author

@digitaldan Implemented!

@TravisBuddy
Copy link

Travis tests were successful

Hey @SRGDamia1,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

2 similar comments
@TravisBuddy
Copy link

Travis tests were successful

Hey @SRGDamia1,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

@TravisBuddy
Copy link

Travis tests were successful

Hey @SRGDamia1,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

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 saw that you put a lot of effort into this binding. Please take a look if you (or any user) really need all the trace log messages or if they can be replaced by using the debugger.

Comment on lines 132 to 133
| shadeChannels | openShade | Switch | Opens or closes motorize shades | Motor Shades |
| shadeChannels | hold | Switch | Tells a device to stop moving | Motor Shades |
Copy link
Member

Choose a reason for hiding this comment

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

Did you consider using the Rollershutter item-type?

Copy link
Author

Choose a reason for hiding this comment

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

Yes and there was some reason I decided not to use it. I don't remember why. I'll look into it again.

return packetIsDuplicate;
}

private void datagramSocketHealthRoutine() {
Copy link
Member

Choose a reason for hiding this comment

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

You should add a grace period of a few seconds here, to prevent hammering on the network (and local resources) when the error is persistent.

Copy link
Author

Choose a reason for hiding this comment

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

I'm sorry. What's the best way to implement that?

Copy link
Member

Choose a reason for hiding this comment

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

Since you run in your own thread, you can simply use Thread.sleep().

@SRGDamia1 SRGDamia1 requested a review from a team as a code owner June 7, 2020 04:37
@TravisBuddy
Copy link

Travis tests have failed

Hey @SRGDamia1,
please read the following log in order to understand the failure reason. There might also be some helpful tips along the way.
It will be awesome if you fix what is wrong and commit the changes.

3 similar comments
@TravisBuddy
Copy link

Travis tests have failed

Hey @SRGDamia1,
please read the following log in order to understand the failure reason. There might also be some helpful tips along the way.
It will be awesome if you fix what is wrong and commit the changes.

@TravisBuddy
Copy link

Travis tests have failed

Hey @SRGDamia1,
please read the following log in order to understand the failure reason. There might also be some helpful tips along the way.
It will be awesome if you fix what is wrong and commit the changes.

@TravisBuddy
Copy link

Travis tests have failed

Hey @SRGDamia1,
please read the following log in order to understand the failure reason. There might also be some helpful tips along the way.
It will be awesome if you fix what is wrong and commit the changes.

@SRGDamia1
Copy link
Author

Thank you so much for your time. I'm sorry I've been really slow to respond. And, obviously, java isn't my native tongue.

I've fixed the quick and easy things.

When implement asynchronous http requests, can I still make the function call "synchronous" so that no two requests to the bridge can overlap? That is, can I make sure I got a response to the last request before sending a new one?

@TravisBuddy
Copy link

Travis tests were successful

Hey @SRGDamia1,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

1 similar comment
@TravisBuddy
Copy link

Travis tests were successful

Hey @SRGDamia1,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

@fwolter
Copy link
Member

fwolter commented Jun 7, 2020

When implement asynchronous http requests, can I still make the function call "synchronous" so that no two requests to the bridge can overlap?

Yes, you can do that, but I don't think this is natively supported by HttpClient. So, you have to write the synchronization code yourself. Using async HTTP requests is not a requirement. You could stick to your current implementation, if network timeouts are kept low (<=10sec). The disadavantage is blocking a framework's scheduler thread as long as the code waits for the remote (or the network). But many bindings do it this way.

@TravisBuddy
Copy link

Travis tests were successful

Hey @SRGDamia1,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

@TravisBuddy
Copy link

Travis tests have failed

Hey @SRGDamia1,
please read the following log in order to understand the failure reason. There might also be some helpful tips along the way.
It will be awesome if you fix what is wrong and commit the changes.

@TravisBuddy
Copy link

Travis tests were successful

Hey @SRGDamia1,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

@openhab-bot
Copy link
Collaborator

Automatic code migration to openHAB 3 succeeded.

The resulting code can be found at https://ci.openhab.org/job/openHAB-Addons-Migration/78/artifact/bundles/.

You can download the migrated code from there and create a new PR against the master branch of the openhab-addons repo to contribute it for being included in openHAB 3.x.

Please see this issue about the details on how to proceed with your existing PR.

@kaikreuzer
Copy link
Member

If I want to make the changes by hand, is there a proper way to merge/rebase/cherry pick commits onto the new shrunken repo?

This indeed need to be brought into a non-conflicting state. The easiest might be to check out the new 2.5.x branch, copy your code to it and do a single commit. Just name your branch bondhome (rename your old one first to have no name clash) and do a force push to Github.

@SRGDamia1 SRGDamia1 changed the base branch from 2.5.x to main December 28, 2020 02:01
Mostly new null checks

Signed-off-by: Sara Damiano <[email protected]>
Signed-off-by: Sara Damiano <[email protected]>
Signed-off-by: Sara Damiano <[email protected]>
Copy link
Member

@Hilbrand Hilbrand left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution. The code looks great, I've made some general comments and some openHAB specific comments.
Additional, The readme needs to be written and please remove the jar file from this pr. If you want to provide a jar you can make a release on your GitHub fork of openHAB. This can be done on a branch. You can attach the jar file to this release. Than point people to this release tag page (not the jar). This gives a consist url. You can later edit the release and add an updated jar, without having to give a new url.

Also run mvn spotless:apply on your binding to auto-format the code, and also auto update the copyright year. (You might need to rebase to the latest code of this repo).

@NonNullByDefault
@Component(configurationPid = "binding.bondhome", service = ThingHandlerFactory.class)
public class BondHomeHandlerFactory extends BaseThingHandlerFactory {
private Logger logger = LoggerFactory.getLogger(BondHomeHandlerFactory.class);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
private Logger logger = LoggerFactory.getLogger(BondHomeHandlerFactory.class);
private final Logger logger = LoggerFactory.getLogger(BondHomeHandlerFactory.class);

private final Logger logger = LoggerFactory.getLogger(BPUPListener.class);

// To parse the JSON responses
private Gson gsonBuilder;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
private Gson gsonBuilder;
private final Gson gsonBuilder;


public @Nullable String lastRequestId;
private long timeOfLastKeepAlivePacket;
private Boolean shutdown;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
private Boolean shutdown;
private boolean shutdown;

// State Variables
// power: (integer) 1 = on, 0 = off
// Actions
TurnOn("TurnOn", CHANNEL_GROUP_COMMON, CHANNEL_POWER_STATE),
Copy link
Member

@Hilbrand Hilbrand Sep 4, 2020

Choose a reason for hiding this comment

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

Enum's should be uppercase:

Suggested change
TurnOn("TurnOn", CHANNEL_GROUP_COMMON, CHANNEL_POWER_STATE),
TURN_ON("TurnOn", CHANNEL_GROUP_COMMON, CHANNEL_POWER_STATE),

*
* @author Kai Kreuzer - Initial contribution
*/
@Component(service = MDNSDiscoveryParticipant.class, immediate = true)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
@Component(service = MDNSDiscoveryParticipant.class, immediate = true)
@Component(service = MDNSDiscoveryParticipant.class, configurationPid="discovery.mdns.bondhome")

logger.info("Current version is {}, prior version was {}.", CURRENT_BINDING_VERSION, lastBindingVersion);

// Update the thing with the new property value
final Map<String, String> newProperties = new HashMap<>(thing.getProperties());
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
final Map<String, String> newProperties = new HashMap<>(thing.getProperties());
final Map<String, String> newProperties = editProperties();

Comment on lines +725 to +727
final ThingBuilder thingBuilder = editThing();
thingBuilder.withProperties(newProperties);
updateThing(thingBuilder.build());
Copy link
Member

Choose a reason for hiding this comment

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

Why not updateProperties(newProperties) here?


private synchronized boolean wasThingUpdatedExternally(BondDevice devInfo) {
// Check if the Bond hash tree has changed
config = getConfigAs(BondDeviceConfiguration.class);
Copy link
Member

Choose a reason for hiding this comment

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

When the config changes initialize is called again and that already sets the config. Therefor it's not needed to set config again here? Or did I miss some behavior here?

if (api == null) {
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
"Bridge API not available");
initialize();
Copy link
Member

Choose a reason for hiding this comment

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

It should not be needed to call initialize here. If some code in initialize needs to be called this should be put into a separate method. initialize is generally called when the thing itself is initialized by openHAB. So the binding should not do that itself.

logger.trace("Update Time for {}: {}", this.getThing().getLabel(), (new DateTimeType()).toFullString());

updateState(CHANNEL_POWER_STATE, updateState.power == 0 ? OnOffType.OFF : OnOffType.ON);
updateState("timer", new DecimalType(updateState.timer));
Copy link
Member

Choose a reason for hiding this comment

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

Put timer in a constant, like the other channels?

@cpmeister cpmeister added the awaiting feedback Awaiting feedback from the pull request author label Feb 7, 2021
@hmerk hmerk added the stale As soon as a PR is marked stale, it can be removed 6 months later. label Aug 11, 2021
@kaikreuzer
Copy link
Member

No feedback since January, so I guess we should close this. If anyone picks it up in future, I'm happy to re-open the PR again.

@kaikreuzer kaikreuzer closed this Sep 20, 2021
ccutrer added a commit to ccutrer/openhab-addons that referenced this pull request Sep 29, 2022
@openhab-bot
Copy link
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/bond-home-bondhome-io/46654/62

jlaur pushed a commit that referenced this pull request Dec 5, 2022
* First commit on newly created branch, taking code from c8b8e21
* [bondhome] update snapshot version, and some typos
* [bondhome] Address (most) comments from prior review from #7260
* [bondhome] simplify channels

 * lastUpdate is unnecessary; turn on persistence or add a rule on update if
   you care to keep track of it
 * use a single string command channel for all shoot-and-forget commands, instead
   of multiple switch channels
 * use a rollershutter channel for shades (accepting UP, DOWN, STOP, 0%, and 100%)
 * on all dimmer channels, accept ON and OFF, as well as 0% to imply OFF, instead
   of having to write rules to control ON/OFF state separately.
 * if the dimmer channel exists, prune the corresponding power channel, since
   the dimmer channel is now a pure superset of its functionality
 * overload fan#speed to be ceiling fan or a fireplace's fan, depending on the
   device type
* [bondhome] add bundle to the BOM pom
* [bondhome] clean up BondDeviceHandler a bit

 * there's no need to delay initialization; ThingManager won't
   even attempt to initialize a child thing until its bridge is online
 * Remove some extra initialization checks that can never be false
 * slightly refactor some methods to return early, rather than
   nest a giant `else`
 * remove some info logging that will get triggered in normal usage
* [bondhome] fix bridge discovery

 * Bridge property and config serial number need to be the same name
 * Don't arbitrarily delay the BPUPListener
 * Automatically update the IP if the BPUPListener finds it
 * Provide the new bridge with its discovered IP to avoid an additional
   DNS query
 * Don't get the bridge version after every keep-alive response
* [bondhome] trigger end-device discovery as soon as the bridge comes online
* [bondhome] remove internal binding version
* [bondhome] change addr property to string

Certain values seen in the wild when interpreted as a long are too big for that
storage. Also, the Bond API documentation describes the addr property on a
device to be a string.

OpenHAB already has infrastructure to have things update their
channel definitions when a binding is updated.

* [bondhome] ignore any device that starts with _

In v3 of their API, Bond added a new special entry of __. Because no valid
device id would start with an underscore, ignore everything that starts with an
underscore to fix v3 and maybe futureproof.

* address review comments

mostly adding i18n to error states, and cleaning up error handling of
HTTP requests.

* use builtin translation services

instead of plumbing our own provider through

* use System.nanoTime instead of currentTimeMillis

so that it will be a monotonic clock, not (as) susceptible to the clock
changing

* [bondhome] ignore BPUP messasges that aren't state

In recent firmware, bond is now sending action messages via BPUP as well as state.
This change ignores all messages that aren't state.

* [bondhome] Improve error handling, and remove dummy constants

Just use a single BondException class to communicate any sort of
error from within bond, and avoid throwing, catching, and re-throwing
the same (or slightly modified) exception.

Also remove dummy constants that might give the wrong impression
of the details of your Bond device. Then implement proper null checks,
especially setting a configuration error if key config properties
aren't set on the thing.

* [bondhome] avoid setting device status when bridge just went offline

* address static analysis tool problems

Also-by: Sara Damiano <[email protected]>
Also-by: Keith T. Garner <[email protected]>
Signed-off-by: Cody Cutrer <[email protected]>
morph166955 pushed a commit to morph166955/openhab-addons that referenced this pull request Dec 18, 2022
* First commit on newly created branch, taking code from c8b8e21
* [bondhome] update snapshot version, and some typos
* [bondhome] Address (most) comments from prior review from openhab#7260
* [bondhome] simplify channels

 * lastUpdate is unnecessary; turn on persistence or add a rule on update if
   you care to keep track of it
 * use a single string command channel for all shoot-and-forget commands, instead
   of multiple switch channels
 * use a rollershutter channel for shades (accepting UP, DOWN, STOP, 0%, and 100%)
 * on all dimmer channels, accept ON and OFF, as well as 0% to imply OFF, instead
   of having to write rules to control ON/OFF state separately.
 * if the dimmer channel exists, prune the corresponding power channel, since
   the dimmer channel is now a pure superset of its functionality
 * overload fan#speed to be ceiling fan or a fireplace's fan, depending on the
   device type
* [bondhome] add bundle to the BOM pom
* [bondhome] clean up BondDeviceHandler a bit

 * there's no need to delay initialization; ThingManager won't
   even attempt to initialize a child thing until its bridge is online
 * Remove some extra initialization checks that can never be false
 * slightly refactor some methods to return early, rather than
   nest a giant `else`
 * remove some info logging that will get triggered in normal usage
* [bondhome] fix bridge discovery

 * Bridge property and config serial number need to be the same name
 * Don't arbitrarily delay the BPUPListener
 * Automatically update the IP if the BPUPListener finds it
 * Provide the new bridge with its discovered IP to avoid an additional
   DNS query
 * Don't get the bridge version after every keep-alive response
* [bondhome] trigger end-device discovery as soon as the bridge comes online
* [bondhome] remove internal binding version
* [bondhome] change addr property to string

Certain values seen in the wild when interpreted as a long are too big for that
storage. Also, the Bond API documentation describes the addr property on a
device to be a string.

OpenHAB already has infrastructure to have things update their
channel definitions when a binding is updated.

* [bondhome] ignore any device that starts with _

In v3 of their API, Bond added a new special entry of __. Because no valid
device id would start with an underscore, ignore everything that starts with an
underscore to fix v3 and maybe futureproof.

* address review comments

mostly adding i18n to error states, and cleaning up error handling of
HTTP requests.

* use builtin translation services

instead of plumbing our own provider through

* use System.nanoTime instead of currentTimeMillis

so that it will be a monotonic clock, not (as) susceptible to the clock
changing

* [bondhome] ignore BPUP messasges that aren't state

In recent firmware, bond is now sending action messages via BPUP as well as state.
This change ignores all messages that aren't state.

* [bondhome] Improve error handling, and remove dummy constants

Just use a single BondException class to communicate any sort of
error from within bond, and avoid throwing, catching, and re-throwing
the same (or slightly modified) exception.

Also remove dummy constants that might give the wrong impression
of the details of your Bond device. Then implement proper null checks,
especially setting a configuration error if key config properties
aren't set on the thing.

* [bondhome] avoid setting device status when bridge just went offline

* address static analysis tool problems

Also-by: Sara Damiano <[email protected]>
Also-by: Keith T. Garner <[email protected]>
Signed-off-by: Cody Cutrer <[email protected]>
Signed-off-by: Ben Rosenblum <[email protected]>
andrasU pushed a commit to andrasU/openhab-addons that referenced this pull request Dec 24, 2022
* First commit on newly created branch, taking code from c8b8e21
* [bondhome] update snapshot version, and some typos
* [bondhome] Address (most) comments from prior review from openhab#7260
* [bondhome] simplify channels

 * lastUpdate is unnecessary; turn on persistence or add a rule on update if
   you care to keep track of it
 * use a single string command channel for all shoot-and-forget commands, instead
   of multiple switch channels
 * use a rollershutter channel for shades (accepting UP, DOWN, STOP, 0%, and 100%)
 * on all dimmer channels, accept ON and OFF, as well as 0% to imply OFF, instead
   of having to write rules to control ON/OFF state separately.
 * if the dimmer channel exists, prune the corresponding power channel, since
   the dimmer channel is now a pure superset of its functionality
 * overload fan#speed to be ceiling fan or a fireplace's fan, depending on the
   device type
* [bondhome] add bundle to the BOM pom
* [bondhome] clean up BondDeviceHandler a bit

 * there's no need to delay initialization; ThingManager won't
   even attempt to initialize a child thing until its bridge is online
 * Remove some extra initialization checks that can never be false
 * slightly refactor some methods to return early, rather than
   nest a giant `else`
 * remove some info logging that will get triggered in normal usage
* [bondhome] fix bridge discovery

 * Bridge property and config serial number need to be the same name
 * Don't arbitrarily delay the BPUPListener
 * Automatically update the IP if the BPUPListener finds it
 * Provide the new bridge with its discovered IP to avoid an additional
   DNS query
 * Don't get the bridge version after every keep-alive response
* [bondhome] trigger end-device discovery as soon as the bridge comes online
* [bondhome] remove internal binding version
* [bondhome] change addr property to string

Certain values seen in the wild when interpreted as a long are too big for that
storage. Also, the Bond API documentation describes the addr property on a
device to be a string.

OpenHAB already has infrastructure to have things update their
channel definitions when a binding is updated.

* [bondhome] ignore any device that starts with _

In v3 of their API, Bond added a new special entry of __. Because no valid
device id would start with an underscore, ignore everything that starts with an
underscore to fix v3 and maybe futureproof.

* address review comments

mostly adding i18n to error states, and cleaning up error handling of
HTTP requests.

* use builtin translation services

instead of plumbing our own provider through

* use System.nanoTime instead of currentTimeMillis

so that it will be a monotonic clock, not (as) susceptible to the clock
changing

* [bondhome] ignore BPUP messasges that aren't state

In recent firmware, bond is now sending action messages via BPUP as well as state.
This change ignores all messages that aren't state.

* [bondhome] Improve error handling, and remove dummy constants

Just use a single BondException class to communicate any sort of
error from within bond, and avoid throwing, catching, and re-throwing
the same (or slightly modified) exception.

Also remove dummy constants that might give the wrong impression
of the details of your Bond device. Then implement proper null checks,
especially setting a configuration error if key config properties
aren't set on the thing.

* [bondhome] avoid setting device status when bridge just went offline

* address static analysis tool problems

Also-by: Sara Damiano <[email protected]>
Also-by: Keith T. Garner <[email protected]>
Signed-off-by: Cody Cutrer <[email protected]>
Signed-off-by: Andras Uhrin <[email protected]>
borazslo pushed a commit to borazslo/openhab-mideaac-addon that referenced this pull request Jan 8, 2023
* First commit on newly created branch, taking code from c8b8e21
* [bondhome] update snapshot version, and some typos
* [bondhome] Address (most) comments from prior review from openhab#7260
* [bondhome] simplify channels

 * lastUpdate is unnecessary; turn on persistence or add a rule on update if
   you care to keep track of it
 * use a single string command channel for all shoot-and-forget commands, instead
   of multiple switch channels
 * use a rollershutter channel for shades (accepting UP, DOWN, STOP, 0%, and 100%)
 * on all dimmer channels, accept ON and OFF, as well as 0% to imply OFF, instead
   of having to write rules to control ON/OFF state separately.
 * if the dimmer channel exists, prune the corresponding power channel, since
   the dimmer channel is now a pure superset of its functionality
 * overload fan#speed to be ceiling fan or a fireplace's fan, depending on the
   device type
* [bondhome] add bundle to the BOM pom
* [bondhome] clean up BondDeviceHandler a bit

 * there's no need to delay initialization; ThingManager won't
   even attempt to initialize a child thing until its bridge is online
 * Remove some extra initialization checks that can never be false
 * slightly refactor some methods to return early, rather than
   nest a giant `else`
 * remove some info logging that will get triggered in normal usage
* [bondhome] fix bridge discovery

 * Bridge property and config serial number need to be the same name
 * Don't arbitrarily delay the BPUPListener
 * Automatically update the IP if the BPUPListener finds it
 * Provide the new bridge with its discovered IP to avoid an additional
   DNS query
 * Don't get the bridge version after every keep-alive response
* [bondhome] trigger end-device discovery as soon as the bridge comes online
* [bondhome] remove internal binding version
* [bondhome] change addr property to string

Certain values seen in the wild when interpreted as a long are too big for that
storage. Also, the Bond API documentation describes the addr property on a
device to be a string.

OpenHAB already has infrastructure to have things update their
channel definitions when a binding is updated.

* [bondhome] ignore any device that starts with _

In v3 of their API, Bond added a new special entry of __. Because no valid
device id would start with an underscore, ignore everything that starts with an
underscore to fix v3 and maybe futureproof.

* address review comments

mostly adding i18n to error states, and cleaning up error handling of
HTTP requests.

* use builtin translation services

instead of plumbing our own provider through

* use System.nanoTime instead of currentTimeMillis

so that it will be a monotonic clock, not (as) susceptible to the clock
changing

* [bondhome] ignore BPUP messasges that aren't state

In recent firmware, bond is now sending action messages via BPUP as well as state.
This change ignores all messages that aren't state.

* [bondhome] Improve error handling, and remove dummy constants

Just use a single BondException class to communicate any sort of
error from within bond, and avoid throwing, catching, and re-throwing
the same (or slightly modified) exception.

Also remove dummy constants that might give the wrong impression
of the details of your Bond device. Then implement proper null checks,
especially setting a configuration error if key config properties
aren't set on the thing.

* [bondhome] avoid setting device status when bridge just went offline

* address static analysis tool problems

Also-by: Sara Damiano <[email protected]>
Also-by: Keith T. Garner <[email protected]>
Signed-off-by: Cody Cutrer <[email protected]>
psmedley pushed a commit to psmedley/openhab-addons that referenced this pull request Feb 23, 2023
* First commit on newly created branch, taking code from c8b8e21
* [bondhome] update snapshot version, and some typos
* [bondhome] Address (most) comments from prior review from openhab#7260
* [bondhome] simplify channels

 * lastUpdate is unnecessary; turn on persistence or add a rule on update if
   you care to keep track of it
 * use a single string command channel for all shoot-and-forget commands, instead
   of multiple switch channels
 * use a rollershutter channel for shades (accepting UP, DOWN, STOP, 0%, and 100%)
 * on all dimmer channels, accept ON and OFF, as well as 0% to imply OFF, instead
   of having to write rules to control ON/OFF state separately.
 * if the dimmer channel exists, prune the corresponding power channel, since
   the dimmer channel is now a pure superset of its functionality
 * overload fan#speed to be ceiling fan or a fireplace's fan, depending on the
   device type
* [bondhome] add bundle to the BOM pom
* [bondhome] clean up BondDeviceHandler a bit

 * there's no need to delay initialization; ThingManager won't
   even attempt to initialize a child thing until its bridge is online
 * Remove some extra initialization checks that can never be false
 * slightly refactor some methods to return early, rather than
   nest a giant `else`
 * remove some info logging that will get triggered in normal usage
* [bondhome] fix bridge discovery

 * Bridge property and config serial number need to be the same name
 * Don't arbitrarily delay the BPUPListener
 * Automatically update the IP if the BPUPListener finds it
 * Provide the new bridge with its discovered IP to avoid an additional
   DNS query
 * Don't get the bridge version after every keep-alive response
* [bondhome] trigger end-device discovery as soon as the bridge comes online
* [bondhome] remove internal binding version
* [bondhome] change addr property to string

Certain values seen in the wild when interpreted as a long are too big for that
storage. Also, the Bond API documentation describes the addr property on a
device to be a string.

OpenHAB already has infrastructure to have things update their
channel definitions when a binding is updated.

* [bondhome] ignore any device that starts with _

In v3 of their API, Bond added a new special entry of __. Because no valid
device id would start with an underscore, ignore everything that starts with an
underscore to fix v3 and maybe futureproof.

* address review comments

mostly adding i18n to error states, and cleaning up error handling of
HTTP requests.

* use builtin translation services

instead of plumbing our own provider through

* use System.nanoTime instead of currentTimeMillis

so that it will be a monotonic clock, not (as) susceptible to the clock
changing

* [bondhome] ignore BPUP messasges that aren't state

In recent firmware, bond is now sending action messages via BPUP as well as state.
This change ignores all messages that aren't state.

* [bondhome] Improve error handling, and remove dummy constants

Just use a single BondException class to communicate any sort of
error from within bond, and avoid throwing, catching, and re-throwing
the same (or slightly modified) exception.

Also remove dummy constants that might give the wrong impression
of the details of your Bond device. Then implement proper null checks,
especially setting a configuration error if key config properties
aren't set on the thing.

* [bondhome] avoid setting device status when bridge just went offline

* address static analysis tool problems

Also-by: Sara Damiano <[email protected]>
Also-by: Keith T. Garner <[email protected]>
Signed-off-by: Cody Cutrer <[email protected]>
nemerdaud pushed a commit to nemerdaud/openhab-addons that referenced this pull request Feb 28, 2023
* First commit on newly created branch, taking code from c8b8e21
* [bondhome] update snapshot version, and some typos
* [bondhome] Address (most) comments from prior review from openhab#7260
* [bondhome] simplify channels

 * lastUpdate is unnecessary; turn on persistence or add a rule on update if
   you care to keep track of it
 * use a single string command channel for all shoot-and-forget commands, instead
   of multiple switch channels
 * use a rollershutter channel for shades (accepting UP, DOWN, STOP, 0%, and 100%)
 * on all dimmer channels, accept ON and OFF, as well as 0% to imply OFF, instead
   of having to write rules to control ON/OFF state separately.
 * if the dimmer channel exists, prune the corresponding power channel, since
   the dimmer channel is now a pure superset of its functionality
 * overload fan#speed to be ceiling fan or a fireplace's fan, depending on the
   device type
* [bondhome] add bundle to the BOM pom
* [bondhome] clean up BondDeviceHandler a bit

 * there's no need to delay initialization; ThingManager won't
   even attempt to initialize a child thing until its bridge is online
 * Remove some extra initialization checks that can never be false
 * slightly refactor some methods to return early, rather than
   nest a giant `else`
 * remove some info logging that will get triggered in normal usage
* [bondhome] fix bridge discovery

 * Bridge property and config serial number need to be the same name
 * Don't arbitrarily delay the BPUPListener
 * Automatically update the IP if the BPUPListener finds it
 * Provide the new bridge with its discovered IP to avoid an additional
   DNS query
 * Don't get the bridge version after every keep-alive response
* [bondhome] trigger end-device discovery as soon as the bridge comes online
* [bondhome] remove internal binding version
* [bondhome] change addr property to string

Certain values seen in the wild when interpreted as a long are too big for that
storage. Also, the Bond API documentation describes the addr property on a
device to be a string.

OpenHAB already has infrastructure to have things update their
channel definitions when a binding is updated.

* [bondhome] ignore any device that starts with _

In v3 of their API, Bond added a new special entry of __. Because no valid
device id would start with an underscore, ignore everything that starts with an
underscore to fix v3 and maybe futureproof.

* address review comments

mostly adding i18n to error states, and cleaning up error handling of
HTTP requests.

* use builtin translation services

instead of plumbing our own provider through

* use System.nanoTime instead of currentTimeMillis

so that it will be a monotonic clock, not (as) susceptible to the clock
changing

* [bondhome] ignore BPUP messasges that aren't state

In recent firmware, bond is now sending action messages via BPUP as well as state.
This change ignores all messages that aren't state.

* [bondhome] Improve error handling, and remove dummy constants

Just use a single BondException class to communicate any sort of
error from within bond, and avoid throwing, catching, and re-throwing
the same (or slightly modified) exception.

Also remove dummy constants that might give the wrong impression
of the details of your Bond device. Then implement proper null checks,
especially setting a configuration error if key config properties
aren't set on the thing.

* [bondhome] avoid setting device status when bridge just went offline

* address static analysis tool problems

Also-by: Sara Damiano <[email protected]>
Also-by: Keith T. Garner <[email protected]>
Signed-off-by: Cody Cutrer <[email protected]>
andrasU pushed a commit to andrasU/openhab-addons that referenced this pull request Jan 6, 2024
* First commit on newly created branch, taking code from c8b8e21
* [bondhome] update snapshot version, and some typos
* [bondhome] Address (most) comments from prior review from openhab#7260
* [bondhome] simplify channels

 * lastUpdate is unnecessary; turn on persistence or add a rule on update if
   you care to keep track of it
 * use a single string command channel for all shoot-and-forget commands, instead
   of multiple switch channels
 * use a rollershutter channel for shades (accepting UP, DOWN, STOP, 0%, and 100%)
 * on all dimmer channels, accept ON and OFF, as well as 0% to imply OFF, instead
   of having to write rules to control ON/OFF state separately.
 * if the dimmer channel exists, prune the corresponding power channel, since
   the dimmer channel is now a pure superset of its functionality
 * overload fan#speed to be ceiling fan or a fireplace's fan, depending on the
   device type
* [bondhome] add bundle to the BOM pom
* [bondhome] clean up BondDeviceHandler a bit

 * there's no need to delay initialization; ThingManager won't
   even attempt to initialize a child thing until its bridge is online
 * Remove some extra initialization checks that can never be false
 * slightly refactor some methods to return early, rather than
   nest a giant `else`
 * remove some info logging that will get triggered in normal usage
* [bondhome] fix bridge discovery

 * Bridge property and config serial number need to be the same name
 * Don't arbitrarily delay the BPUPListener
 * Automatically update the IP if the BPUPListener finds it
 * Provide the new bridge with its discovered IP to avoid an additional
   DNS query
 * Don't get the bridge version after every keep-alive response
* [bondhome] trigger end-device discovery as soon as the bridge comes online
* [bondhome] remove internal binding version
* [bondhome] change addr property to string

Certain values seen in the wild when interpreted as a long are too big for that
storage. Also, the Bond API documentation describes the addr property on a
device to be a string.

OpenHAB already has infrastructure to have things update their
channel definitions when a binding is updated.

* [bondhome] ignore any device that starts with _

In v3 of their API, Bond added a new special entry of __. Because no valid
device id would start with an underscore, ignore everything that starts with an
underscore to fix v3 and maybe futureproof.

* address review comments

mostly adding i18n to error states, and cleaning up error handling of
HTTP requests.

* use builtin translation services

instead of plumbing our own provider through

* use System.nanoTime instead of currentTimeMillis

so that it will be a monotonic clock, not (as) susceptible to the clock
changing

* [bondhome] ignore BPUP messasges that aren't state

In recent firmware, bond is now sending action messages via BPUP as well as state.
This change ignores all messages that aren't state.

* [bondhome] Improve error handling, and remove dummy constants

Just use a single BondException class to communicate any sort of
error from within bond, and avoid throwing, catching, and re-throwing
the same (or slightly modified) exception.

Also remove dummy constants that might give the wrong impression
of the details of your Bond device. Then implement proper null checks,
especially setting a configuration error if key config properties
aren't set on the thing.

* [bondhome] avoid setting device status when bridge just went offline

* address static analysis tool problems

Also-by: Sara Damiano <[email protected]>
Also-by: Keith T. Garner <[email protected]>
Signed-off-by: Cody Cutrer <[email protected]>
Signed-off-by: Andras Uhrin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting feedback Awaiting feedback from the pull request author new binding If someone has started to work on a binding. For a new binding PR. stale As soon as a PR is marked stale, it can be removed 6 months later.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants