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.

Sign up for GitHub

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

Jump to bottom

When using manual control the home status is not updated #25

Closed
rpineau opened this issue Dec 7, 2019 · 16 comments
Closed

When using manual control the home status is not updated #25

rpineau opened this issue Dec 7, 2019 · 16 comments
Assignees
@NameOfTheDragon
Labels
bug Something isn't working

Comments

@rpineau
Copy link

rpineau commented Dec 7, 2019

A user pinged me as there is an issue when using both computer control and the rocker switch to move the dome.
when moving the dome via the rocker switch the position is properly updated but the at home status is not.
Here is how the user tested this and what I got from my log :

1. Connect Dome = OK; Note the dome is starting out at home (Az164)

Yes.

2. MANUALLY GoTo Az146 (approximately)
Yes, I see it

3. Find Home = DOME DOES NOT MOVE!! Az changes to 164 and dome reports Finding home complete. No errors <<<===

This is where we see the issue
The position is :
[Sat Dec 7 08:15:03 2019] [CNexDomeV3::getDomeAz] dDomeAz = 145.95

I send the command to get the status :

[Sat Dec 7 08:15:03 2019] [CNexDomeV3::domeCommand] sending : @srr

The response is :
[Sat Dec 7 08:15:03 2019] [CNexDomeV3::readResponse] response = :SER,22331,1,55080,25092,150#

If we decode it :
SER : status response
22331 : position in tick counts
1: At home status , 1 means at home <==== THIS IS NOT TRUE as we moved.

4. Use Command to GoTo Az150 = Dome moves to what it thinks is Az150 which is now actually about Az132 or so

It’s like as you move manually some internal sync is not done and it’s lost !

5. Find Home = Dome goes all the way back to home sensor; Az reports 164 (position is now correct)
Yea, because this time it comes from the serial command so probably goes through a different code path.

So this is clearly a firmware bug when using manual control with computer control at the same time
@NameOfTheDragon NameOfTheDragon self-assigned this Dec 7, 2019
@NameOfTheDragon NameOfTheDragon added the investigating Issue is under investigation to see if can be reproduced label Dec 7, 2019
@NameOfTheDragon
Copy link
Collaborator

I can't initially reproduce this. Is there something special about the numbers or does this happen from any starting position?

@rpineau
Copy link
Author

rpineau commented Dec 7, 2019 via email

@NameOfTheDragon
Copy link
Collaborator

OK thanks.

So looking at the code, it is difficult to see how the homing operation can fail to start.

void HomeSensor::findHome(int direction)
	{
	if (phase == Idle || phase == AtHome)
		{
		const auto distance = 2 * homeSettings->microstepsPerRotation;	// Allow 2 full rotations only
		setPhase(Detecting);
		motor->moveToPosition(distance);
		}
	}

The only possibility I can really see there is that the phase state variable must be in the correct state or homing will not start. Perhaps the state is getting left in a corrupt state from a previous operation. This will need further investigation.

Home detection is via an edge triggered interrupt from the home sensor. Even if this were happening immediately after starting the home operation before the motor has taken even a single step, then that could possibly look like nothing has happened. This seems unlikely though, with a nearly 20 degree movement away from home.

@NameOfTheDragon
Copy link
Collaborator

Nope, I can't make it happen. Is there any possibility of getting the customer to reproduce it in PuTTY or a dumb terminal emulator, so we can see the raw output from the rotator?

@rpineau
Copy link
Author

rpineau commented Dec 7, 2019 via email

@NameOfTheDragon
Copy link
Collaborator

NameOfTheDragon commented Dec 7, 2019 •
edited

I believe it will be set to Idle the next time there is an onMotorStopped event...

/*
 * Handles the onMotorStopped event. Action depends on the homing phase.
 */
void HomeSensor::onMotorStopped() const
	{
	std::cout << "Hstop " << phase << std::endl;
	switch (phase)
		{
		case Reversing:
			setPhase(AtHome);
			break;
		case Stopping:
			setPhase(Reversing);
			const auto target = commandProcessor.targetStepPosition(homeSettings->position);
			motor->moveToPosition(target);
			break;
		default:
			setPhase(Idle);
			return;
		}
	}

Even if that were not the case, it is explicitly allowed to start another homing operation while already at home, so that would still work.

@rpineau
Copy link
Author

rpineau commented Dec 7, 2019 via email

@NameOfTheDragon
Copy link
Collaborator

Ah yes. Thanks for doing that @rpineau , that's really helpful. I will have another go at reproducing it here.

@NameOfTheDragon
Copy link
Collaborator

OK, yes I can reproduce it now. It's pretty odd because the output doesn't seem to match what the code says should happen.

P47885
P47686
P47565
STOP
Hstop 4
:SER,47522,1,55080,0,50#
XB->Online

With this output, we can see STOP (motor stopped event) and then HStop 4. This is produced in the home sensor onMotorStopped event handler. The 4 is "phase 4" which is AtHome (this is expected at thsi point). However, a few lines later the switch statement should set the phase to Idle (since it is known to be AtHome and not Reversing or Stopping but it looks like that's not happening:

/*
 * Handles the onMotorStopped event. Action depends on the homing phase.
 */
void HomeSensor::onMotorStopped() const
	{
	std::cout << "Hstop " << phase << std::endl;
	switch (phase)
		{
		case Reversing:
			setPhase(AtHome);
			break;
		case Stopping:
			setPhase(Reversing);
			const auto target = commandProcessor.targetStepPosition(homeSettings->position);
			motor->moveToPosition(target);
			break;
		default:
			setPhase(Idle);
			return;
		}

Is there some subtlety of C++ that I'm missing that could cause that default case not to execute?

@rpineau
Copy link
Author

rpineau commented Dec 7, 2019 via email

@NameOfTheDragon NameOfTheDragon added bug Something isn't working and removed investigating Issue is under investigation to see if can be reproduced labels Dec 8, 2019
@NameOfTheDragon
Copy link
Collaborator

If this is based on the homing phase (Action depends on the homing phase. in your comment), then what happen if we're not homing.

By design, the state should be AtHome at the end of a FindHome operation and Idle when no homing operation is in progress (it is actually cleared to Idle each time the motor stops, except if the previous state was Reversing). Therefore, it follows that (by design) AtHome will be true at the end of a successful homing operation and false at all other times.

However it looks like there may be a compiler bug that's causing it to generate incorrect code for switch statements. I had something similar elsewhere in the code and I had to re-order my case statements to get it to work. I've re-written it using if statements and its working now. I'll send you a hotfix to test shortly.

Aside: I wouldn't recommend relying on the AtHome flag for anything other than display purposes, because Find Home is an operation, not a position. The AtHome flag has no meaning outside of a Find Home operation and it should never be true except at the end of a successful Find Home operation. There is also no defined "home position" (from the firmware's point of view), there is only the azimuth of the home sensor, which is not quite the same thing, conceptually. The definition is in the FAQ under https://github.com/nexdome/ASCOM/wiki/Home,-Park-and-Home-Sensor-Magnet-Positioning#home-sensor-azimuth.

@NameOfTheDragon
Copy link
Collaborator

@rpineau could you give this a go please?
Rotator-3.2.1-Beta.1.zip

@rpineau
Copy link
Author

rpineau commented Dec 8, 2019 via email •
edited

@NameOfTheDragon
Copy link
Collaborator

NameOfTheDragon commented Dec 9, 2019 via email

@rpineau
Copy link
Author

rpineau commented Dec 25, 2019 via email

@NameOfTheDragon
Copy link
Collaborator

Fixed for firmware release 3.3.0, later today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@NameOfTheDragon NameOfTheDragon

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@rpineau @NameOfTheDragon