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

IPX800v4 update #2210

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

IPX800v4 update #2210

wants to merge 4 commits into from

Conversation

aknotwot
Copy link

  • typo in drivers.xml
  • now APIKey field is optionnal
  • solving issue on parsing of Analogs Inputs and Digital Outputs

company name typo
API key is now optionnal.
+ Correction of JSON parsing for Analog Inputs and Digital outputs.
@@ -74,6 +74,9 @@ bool IPX800::initProperties()
ModelVersionTP.fill(getDeviceName(), "MODEL", "Model", "Main Control", IP_RO, 60, IPS_IDLE);

setDefaultPollingPeriod(1000);

// Ensure properties are updated
updateProperties();
Copy link
Contributor

Choose a reason for hiding this comment

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

We cannot call updateProperties in initProperties

@@ -83,16 +86,15 @@ bool IPX800::updateProperties()
INDI::DefaultDevice::updateProperties();
InputInterface::updateProperties();
OutputInterface::updateProperties();

defineProperty(APIKeyTP);
Copy link
Contributor

Choose a reason for hiding this comment

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

If this property needs to exist all the time, then it needs to be moved to ISGetProperties function instead. Properties defined there exist for the lifetime of the driver regardless of the connection status.

Copy link
Author

Choose a reason for hiding this comment

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

ok, i will check.

Copy link
Author

Choose a reason for hiding this comment

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

My tested proposal. it works.
void IPX800::ISGetProperties(const char *dev)
{

	INDI::DefaultDevice::ISGetProperties(dev);
    //Define the APIKeyTP property
    APIKeyTP[0].fill("API_KEY", "API Key", "");
    APIKeyTP.fill(getDeviceName(), "API_KEY", "API Settings", MAIN_CONTROL_TAB, IP_RW, 60, IPS_IDLE);
	defineProperty(APIKeyTP);	
	APIKeyTP.load();

}

If this modification is ok for you, should i do a new pull request, to merge it ? or is it possible to modify this one ?

Copy link
Contributor

Choose a reason for hiding this comment

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

This should be in initProperties:

APIKeyTP[0].fill("API_KEY", "API Key", "");
APIKeyTP.fill(getDeviceName(), "API_KEY", "API Settings", MAIN_CONTROL_TAB, IP_RW, 60, IPS_IDLE);
APIKeyTP.load();

This should be in ISGetProperties:

INDI::DefaultDevice::ISGetProperties(dev);
defineProperty(APIKeyTP);

Copy link
Author

Choose a reason for hiding this comment

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

Done.

Implementation of ISGetProperties for APIKeyTP declaration.
@aknotwot
Copy link
Author

Commit added (use of ISGetProperties) the pull request

APIKeyTP init moved to initProperties
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants