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

Upgrade CheapoDC driver for new device firmware features #2142

Merged
merged 6 commits into from
Nov 4, 2024

Conversation

hcomet
Copy link
Contributor

@hcomet hcomet commented Oct 30, 2024

CheapoDC driver release to add support for firmware 2.x features
- bug fix for Longitude range checking (actually a firmware bug).
- adds support for Weather Device snoop for local temperature/humidity instead of using CheapoDC weather query
- move Latitude/Longitude settings to more common Site Management Tab with Location settings
- Location Snoop enabled by default for Telescope Simulator
- maintain backwards compatibility with previous firmware version
Weather Simulator
- added Humidity as a weather value so that dew point may be calculated by snooping devices. (ie: CheapoDC)

Includes new ability to snoop weather station device.
Add humidity to Weather Simulator to allow for dew
point calculation.
@hcomet
Copy link
Contributor Author

hcomet commented Oct 30, 2024

I noticed the CheapoDC listing on indilib.org went missing during the website migration so resubmitted.

@knro
Copy link
Contributor

knro commented Oct 30, 2024

I noticed the CheapoDC listing on indilib.org went missing during the website migration so resubmitted.

@naheedsa Please check

@hcomet
Copy link
Contributor Author

hcomet commented Oct 30, 2024

Sorry another question. Looking at the suggested simplification of using Property.load(). There is a call to loadConfig(); in CheapoDC::updateProperties(). I assume the best thing would be to not do any calls to Property.load() in CheapoDC::initProperties(). Also move the couple of EnableSnoop... switch checks to after the loadConfig();

Assuming this would be more correct. Some of what happens when copying other drivers.

@hcomet
Copy link
Contributor Author

hcomet commented Oct 30, 2024

Made the requested simplification changes plus moved the loads to updateProperties.

@hcomet
Copy link
Contributor Author

hcomet commented Oct 30, 2024

Also saw that my driver submission on indilib.org was accepted. I must have done something wrong when submitting the description HTML because it is not rendering properly.

@knro
Copy link
Contributor

knro commented Oct 31, 2024

Property.load does NOT call ISNewXXX if that's what you mean, it simply initializes the property values.

Comment on lines 161 to 167
char locationDevice[MAXINDIDEVICE] = {"Telescope Simulator"};
char locationProperty[MAXINDINAME] = {"GEOGRAPHIC_COORD"};
char locationLatAttribute[MAXINDINAME] = {"LAT"};
char locationLongAttribute[MAXINDINAME] = {"LONG"};
char temperatureDevice[MAXINDIDEVICE] = {"Focuser Simulator"};
char temperatureProperty[MAXINDINAME] = {"FOCUS_TEMPERATURE"};
char temperatureAttribute[MAXINDINAME] = {"TEMPERATURE"};
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you need these for? GEOGRAPHIC_COORD and FOCUS_TEMPERATURE are both standard INDI properties. If you want to snoop other devices, you need to have a single property ActiveDeviceTP for all the different types of devices you need to snoop.

If you check the example above, you'll see how INDI::CCD snoops the properties from other devices.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking back likely not understanding how ActiveDeviceTP works. I will look at it.

Thanks Jasem.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking at INDI:CCD, I see that it uses IUGetConfigText to fill the intial text for the ActiveDeviceTP property with any values that may be found in the config file. This is similar to what I intended above. Moving to using only Property.load(); removes this function. Is there a better or preferred way to do this?

https://github.com/indilib/indi/blob/9d6e87abfddec3d79a4ddf34b0b693f542eb6fb8/libs/indibase/indiccd.cpp#L479C1-L498C1

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right, this code is outdated and need to be removed.

Copy link
Contributor

Choose a reason for hiding this comment

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

INDI::CCD code is updated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to using ActiveDeviceTP property.

Will update the driver listing on indilib.org on Monday.

@knro knro merged commit 4d2d652 into indilib:master Nov 4, 2024
11 checks passed
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