CerebusOSS/CereLink

v7 cbmex thinks that 'fileconfig' is 'comment'

Closed this issue · 10 comments

Dear CereLinkers,

This was a problem with the v6 NSP firmware, too. Just to confirm, do we know if this bug comes from CereLink or if it is inherent in the NSP firmware itself?

'>> cbmex ( 'open' )
Initializing real-time interface 7.00.00.00 (protocol cb3.11)...
Udp real-time interface to NSP (7.00.04.00 hwlib 3.11) successfully initialized
'>> cbmex ( 'time' )

ans =

824.7500

'>> cbmex ( 'fileconfig' , 'd:\test\cerelink_v7' , 'Testing new build of CereLink cbmex.mexa64' , 1 )
Parameter 1 is invalidError using cbmex
Send comment or user event
Format: cbmex('comment', rgba, charset, comment, [[, value]])
Inputs:
rgba: color coding or custom event number
charset: character-set 0 for ASCII or 1 for UTF16 or any user-defined
comment: comment string (maximum 127 characters)
[, value] pairs are optional, some parameters do not have values.
from left to right parameters will override previous ones or combine with them if possible.
[, value] can be any of:
'instance', value: value is the library instance to use (default is 0)'

I think it's just the error message that's wrong. Please modify here, changing CBMEX_FUNCTION_COMMENT to CBMEX_FUNCTION_FILECONFIG, test (i.e. recompile cbmex), then make a pull request.

I think what's triggering the error is the first argument to the function is causing mxGetString(prhs[i], cmdstr, 16) to return something non-zero. Here are the docs for mxGetString. It looks like the 3rd argument is strlen: 'Size in bytes of destination buffer pointed to by str.' And you can get an error if 'strlen is not large enough to store the entire mxArray. If so, then the function returns 1 and truncates the string.'

I don't know how many characters you can fit in the 16-byte string. That might be platform and architecture dependent. You can try increasing this number to see if it works but don't make a PR on that. Just let us know then we will investigate further.

On this line, change 16 to 255. Then on this line, change PARAM_OPTION to PARAM_NONE.

Thanks for the suggestions, I'll let you know when I've had a chance to try 'em!

Some low-level questions:

  1. Will I first modify my local copy of the source code nested in the CereLink-master directory?
  2. To get rid of the old compile, do I: a) delete /usr/local/CereLink, and b) delete CereLink-master/build directories?
  3. To recompile, do I simply run the usual steps from inside a new CereLink-master/build directory?
  4. Or ... must I delete the old CereLink-master directory entirely and unzip a fresh copy?
  5. Shouldn't I also change line 1768 in cbmex.cpp to: char cmdstr[255]; ?

Will I first modify my local copy of the source code nested in the CereLink-master directory?

Using 'CereLink-master' makes me think you downloaded the repository using GitHub's "Download Zip" option, correct? You're about to learn how to contribute to open source software. It might seem daunting at first but you've already proven yourself capable so I'm sure you'll be able to figure it out. I'm going to give you some high-level instructions but it's up to you to figure out how to do it using better resources than me, of which there are many.

Delete your entire CereLink-master directory.
In GitHub, "fork" this repository so you have a copy of it in your own GitHub profile.
Clone your fork to your computer.
In your local clone, make a new branch.
Edit the code.
Test your changes.
When satisified, git add and git commit -m "Some descriptive message about what has changed" and git push.
In your fork in GitHub, create a Pull Request.
We will evaluate it and merge it if appropriate. Your contribution to CereLink will forever be preserved in its git history.

After it's merged, you can delete your fork, or you can leave it up if you think you're going to make more changes in the future. Your fork will not stay up to date automatically. Syncing with the upstream repo is another lesson entirely.

Shouldn't I also change line 1768 in cbmex.cpp to: char cmdstr[255]; ?

Yes!

By the way, there are other places in that file where CBMEX_FUNCTION_COMMENT was used instead of the correct one. If you think you can identify these then please go ahead and make those changes too, otherwise don't worry about it.

Cheers, C. That's just the push I needed to get pulled into Git (or is it the other way around?).

So, the edits you suggest at least lead to sensible error messages.

>> cbmex ( 'fileconfig' , 'd:\test\test2' , 'CereLink fix test' , 1 ) Parameter 3 is invalidError using cbmex Configures file recording Format: [recording filename username] = cbmex('fileconfig', [filename, comments, action, [<parameter>[, value]]]) blah blah blah
But unless I'm missing something, it is currently impossible for prhs[ 3 ] (the 'action' input argument in cbmex PDF documentation) to ever get past that first for loop. It is not mxChar and so an error will be triggered by mxGetString. If I cheat and force the for loop to continue when i == 3 then we get something that appears to function properly.

Now I'm having trouble uploading my branch. This is the outcome:
$ git push --set-upstream origin fix_fileconfig
Username for 'https://github.com': jsdpag
Password for 'https://jsdpag@github.com':
remote: Permission to dashesy/CereLink.git denied to jsdpag.
fatal: unable to access 'https://github.com/dashesy/CereLink.git/': The requested URL returned error: 403

Is this just a permissions issue, or did I use the wrong command?

I will have to look more carefully later, but one thing I noticed: use either 'd:/' or d:\\. Specially '\t' is no good.

Should 'd:\test\test2' be interpreted somewhere as ...
d:<tab>est<tab>est2
?
In my local setup, I do get the Cerebus creating the intended d:\test\test2* files.

remote: Permission to dashesy/CereLink.git denied to jsdpag.

You don't have write access to this repository. That's why you have to first "fork" this repository, so that you have a copy of it under your own name, then you clone/pull/push to your own copy, then from your own copy on GitHub you do a pull request to this repository.

Okay, thanks for clarifying. I thought forking was a synonym for branching, at first.

All right, that's finally done.