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

[atlona] Add support for the AT-PRO3HD66M #9385

Merged
merged 2 commits into from
Dec 25, 2020

Conversation

mlobstein
Copy link
Contributor

Added support for the older (non-UHD) model AT-PRO3HD66M matrix. This device required a slightly different login protocol and selective bypassing of some of the status inquires. This was necessary because sending large amounts of commands to the device that it did not understand would cause it to lock up.

@tmrobert8

Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
@cpmeister cpmeister added the enhancement An enhancement or new feature for an existing add-on label Dec 15, 2020
@@ -192,7 +201,7 @@ String login() throws Exception {
try {
response = callback.getResponse();
if (!response.equals("")) {
logger.info("Altona protocol violation - didn't start with an inital empty response: '{}'", response);
logger.debug("Altona protocol violation - didn't start with an inital empty response: '{}'", response);
}
} catch (Exception e) {
Copy link
Member

Choose a reason for hiding this comment

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

Please do not catch Exception. I assume IOException is enough. Catching Exception also catches NPE and hides programming errors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I completely agree. However in this instance, the exception being caught is explicitly thrown as Exception in getResponse() on lines 1260 & 1262. I could change those to be IOException or a custom exception. What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

I just looked at the rest of the code. This should never have been merged. Removing the Exception here will require some bigger refactoring, which is clearly out of scope here. Leave it as is.

if (!response.equals("")) {
logger.debug("Altona protocol violation - didn't start with an inital empty response: '{}'", response);
}
} catch (Exception e) {
Copy link
Member

Choose a reason for hiding this comment

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

see above

Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
@@ -192,7 +201,7 @@ String login() throws Exception {
try {
response = callback.getResponse();
if (!response.equals("")) {
logger.info("Altona protocol violation - didn't start with an inital empty response: '{}'", response);
logger.debug("Altona protocol violation - didn't start with an inital empty response: '{}'", response);
}
} catch (Exception e) {
Copy link
Member

Choose a reason for hiding this comment

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

I just looked at the rest of the code. This should never have been merged. Removing the Exception here will require some bigger refactoring, which is clearly out of scope here. Leave it as is.

@J-N-K J-N-K merged commit ce69f22 into openhab:main Dec 25, 2020
@J-N-K J-N-K added this to the 3.1 milestone Dec 25, 2020
seaside1 pushed a commit to seaside1/openhab-addons that referenced this pull request Dec 28, 2020
* Add support for the AT-PRO3HD66M

Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
seaside1 pushed a commit to seaside1/openhab-addons that referenced this pull request Dec 28, 2020
* Add support for the AT-PRO3HD66M

Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
Signed-off-by: Joseph Hagberg <joseph@zoidberg.se>
@mlobstein mlobstein deleted the Atlona_additionaldevice branch December 29, 2020 02:08
nowaterman pushed a commit to nowaterman/openhab-addons that referenced this pull request Jan 19, 2021
thinkingstone pushed a commit to thinkingstone/openhab-addons that referenced this pull request Nov 7, 2021
* Add support for the AT-PRO3HD66M

Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
marcfischerboschio pushed a commit to bosch-io/openhab-addons that referenced this pull request May 5, 2022
* Add support for the AT-PRO3HD66M

Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants