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

supernova: use the /error messages to turn on / off the console printing #5820

Merged
merged 5 commits into from
Jul 23, 2022
Merged

supernova: use the /error messages to turn on / off the console printing #5820

merged 5 commits into from
Jul 23, 2022

Conversation

vitreo12
Copy link
Contributor

@vitreo12 vitreo12 commented Jul 22, 2022 •
edited
Loading

Purpose and Motivation

Allow supernova to use the /error message just like scsynth. This was previously being ignored. As discussed on the forum, it is a feature that is needed to match scsynth's capabilities.

This patch does not take into account the local bundles (/error, -1 and /error, -2), as supernova appears to treat the OSC bundles a little differently than scsynth, and I am still studying that part of the code base. This will come as a secondary patch in the future

Types of changes

  • Bug fix

To-do list

  • Code is tested
  • All tests are passing
  • Updated documentation
  • This PR is ready for review

@dyfer
Copy link
Member

dyfer commented Jul 23, 2022

Thanks so much for the fix, @vitreo12
What would you think about leaving a TODO or FIXME comment somewhere in the code indicating the need to implement the local error suppression (-1/-2) in the future? It's just something to consider, not a strong request.

@vitreo12
Copy link
Contributor Author

vitreo12 commented Jul 23, 2022 •
edited
Loading

I thought about it, but then I saw that there already is a comment about it a couple of lines above:

supercollider/server/supernova/sc/sc_osc_handler.hpp

Lines 276 to 279 in 0aa48ad

/** \todo how to handle temporary message error suppression? */
void set_error_posting(int val) { error_posting = val; }

Perhaps it could be more specific, I could surely modify it to make it more clear

@dyfer
Copy link
Member

dyfer commented Jul 23, 2022

Perhaps it could be more specific, I could surely modify it to make it more clear

Ah, I see. We can leave it as is - unless you feel like making the message more specific.

@vitreo12
Copy link
Contributor Author

vitreo12 commented Jul 23, 2022 •
edited
Loading

I agree, we can just leave it as is. I will also soon work on a secondary PR on the local error suppression.

dyfer
dyfer approved these changes Jul 23, 2022
View reviewed changes
Copy link
Member

@dyfer dyfer left a comment

Choose a reason for hiding this comment

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

Thanks @vitreo12
I tested it and it works as expected.

@dyfer dyfer merged commit ecbd988 into supercollider:develop Jul 23, 2022
mtmccrea pushed a commit to mtmccrea/supercollider that referenced this pull request Aug 15, 2022
@vitreo12 @mtmccrea
supernova: use the /error messages to turn on / off the console pri…
85b855d 
…nting (supercollider#5820)

* supernova: retrieve /error value to print to console
@Spacechild1
Copy link
Contributor

Spacechild1 commented Sep 17, 2022 •
edited
Loading

@vitreo12 @dyfer I don't think this is right. The /error message is supposed to disable error messages, but now it disables all console printout.

I think error_posting should be checked in the OSC method handlers instead. This is how it is done in scsynth:

supercollider/server/scsynth/SC_Lib.cpp

Lines 184 to 207 in b9a5700

SCErr SC_LibCmd::Perform(struct World* inWorld, int inSize, char* inData, ReplyAddress* inReply) {
SCErr err;
// int kSendError = 1; // i.e., 0x01 | 0x02;
try {
err = (mFunc)(inWorld, inSize, inData, inReply);
} catch (int iexc) {
err = iexc;
} catch (std::exception& exc) {
if (inWorld->mLocalErrorNotification <= 0 && inWorld->mErrorNotification) {
CallSendFailureCommand(inWorld, (char*)Name(), exc.what(), inReply);
scprintf("FAILURE IN SERVER %s %s\n", (char*)Name(), exc.what());
}
return kSCErr_Failed;
} catch (...) {
err = kSCErr_Failed;
}
if (err && (inWorld->mLocalErrorNotification <= 0 && inWorld->mErrorNotification)) {
char errstr[128];
SC_ErrorString(err, errstr);
CallSendFailureCommand(inWorld, (char*)Name(), errstr, inReply);
scprintf("FAILURE IN SERVER %s %s\n", (char*)Name(), errstr);
}
return err;
}

As a side note, the /error command also takes negative values to disable error messages locally. In Supernova, this is currently not implemented at all. See

supercollider/server/scsynth/SC_MiscCmds.cpp

Lines 1837 to 1861 in b9a5700

SCErr meth_error(World* inWorld, int inSize, char* inData, ReplyAddress* inReply);
SCErr meth_error(World* inWorld, int inSize, char* inData, ReplyAddress* /*inReply*/) {
sc_msg_iter msg(inSize, inData);
int mode = msg.geti();
// inWorld->mLocalErrorNotification = mode;
// // if non-zero, new state should be saved permanently
// if(mode) { inWorld->mErrorNotification = mode; };
// -1 = bundle off, -2 = bundle on, 0 = permanent off, 1 = permanent on
switch (mode) {
case -1:
inWorld->mLocalErrorNotification += 1;
break;
case -2:
inWorld->mLocalErrorNotification -= 1;
break;
case 0:
inWorld->mErrorNotification = 0;
break;
case 1:
inWorld->mErrorNotification = 1;
};
return kSCErr_None;
}
.

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

@dyfer dyfer dyfer approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vitreo12 @dyfer @Spacechild1