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

Check iExit when trying to connect to objects #200

Merged
merged 10 commits into from
Jan 24, 2020

Conversation

konglobemeralt
Copy link
Member

@konglobemeralt konglobemeralt commented Jan 14, 2020

Big change in connection loop in OC

Copy link
Collaborator

@LukasWikander LukasWikander left a comment

Choose a reason for hiding this comment

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

This change does not fix the problem of ObjectControl being stuck in "Was not able to connect to object, retry in 3 sec...". (for a test, try e.g. not starting a VO on the specified IP address and then pressing connect, then shutting down the GUC and ctrl-C the server)

@konglobemeralt konglobemeralt changed the title switched from util_error to log error for connection problems Check iExit when trying to connect to objects Jan 20, 2020
Copy link
Contributor

@viktorjo viktorjo left a comment

Choose a reason for hiding this comment

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

I think that a few changes should be applied before approving. I agree with the statement that we should do something more than reporting, however I would like it if we could have the ability of canceling the connection to objects after this pull-request is done.

server/src/objectcontrol.c Outdated Show resolved Hide resolved
@@ -872,7 +872,7 @@ void objectcontrol_task(TimeType * GPSTime, GSDType * GSD, LOG_LEVEL logLevel) {

}

} while (iResult < 0 && DisconnectU8 == 0);
} while (iExit == 0 && iResult < 0 && DisconnectU8 == 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

In this do-while loop I think that the procedure should a bit different. I think that the for-loop on line 814 should be included inside of this, so that in each iteration of the do-while, the program checks all objects and reports which could not be connected. If this is implemented together with the removal of the sleep function, then the user will be able to at least disconnect from all objects.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you mean we should have it the other way around? Right now the for-loop is inside of the do-while

Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't see that the for-loop was that large. We cannot change this properly now, but we have to change that in the future.

Copy link
Member Author

Choose a reason for hiding this comment

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

have you added a new task? Can add this otherwise.

Copy link
Contributor

Choose a reason for hiding this comment

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

I have it on my todo list :) I will do it.

@viktorjo
Copy link
Contributor

This pull request seems to be stuck in format checking on jenkins, for some reason it fails. Could be worth investigating.

@alfaro01
Copy link
Contributor

alfaro01 commented Jan 21, 2020

This pull request seems to be stuck in format checking on jenkins, for some reason it fails. Could be worth investigating.

@viktorjo This is an issue we have on every branch either I or Jesper pushes. I think Lukas has some settings configured to his Indent version which Jenkins relies on in order to accept the format. Me and Lukas looked at this yesterday and hopefully we can fix this soon.

@konglobemeralt konglobemeralt merged commit 00b2ec5 into dev Jan 24, 2020
@konglobemeralt konglobemeralt deleted the feature_OCconnectionFixes branch January 24, 2020 14:55
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.

4 participants