Skip to content
Open
Show file tree
Hide file tree
Changes from 52 commits
Commits
Show all changes
93 commits
Select commit Hold shift + click to select a range
bfad65f
Merge remote-tracking branch 'upstream/master' into 0.10-libftdi1
Dec 2, 2018
78cc4b9
Added overload of Write() with void Write(ola::io::ByteString)
Dec 15, 2018
e1869e3
Indicate that RDM is supported
Dec 15, 2018
2b5a5fa
Initial attempt at adding queue for RDM commands, also added destruct…
Dec 15, 2018
3b6cb83
This a totally disgusting in-between state of the code, I'm about to …
Dec 19, 2018
c51928e
Initial compiling version that implements a RDM discoverable target f…
Dec 19, 2018
10fc5a0
Removed qtcreator files, added to .gitignore and added minor debug ou…
Dec 19, 2018
1bdf897
Simplified RDM implementation only implement the discovery interface
Dec 28, 2018
71bd693
Merge remote-tracking branch 'upstream/master' into ftdi-rdm
Jan 6, 2019
8553598
Another attempt with no fruits...
Jan 8, 2019
81d120b
Added discovery functions to port, now stuff actually gets called.
Jan 8, 2019
8914f56
Some debug output plus detect that less bytes were written then reque…
Jan 8, 2019
9e95824
Some logic fixes that came up now that discovery actually tries to run.
Jan 8, 2019
ff09297
Clean up Port and remove functions not needed.
Jan 8, 2019
851401b
Minor cosmetic change
Jan 8, 2019
1d09fe1
Added some of the RDMReply callback logic.
Jan 8, 2019
b02363b
Set the pending request back to nullptr in the right places and nome …
Jan 8, 2019
c87ebf6
Added FtdiInterface::WriteAndRead() in an attempt to rule out context…
Jan 10, 2019
0141ad0
3 Fixes
Jan 17, 2019
2373f60
Added single function to handle destruction of any outstanding callba…
Jan 27, 2019
2198e3d
Added 'destruction' of callbacks to destructor.
Jan 27, 2019
65e11d8
Switch back to scheduling with seperate write() and read() functions.
Feb 25, 2019
d719f6b
Removal of WriteAndRead() and related scheduler.
Feb 25, 2019
5645a01
Merge remote-tracking branch 'upstream/master' into ftdi-rdm
Feb 25, 2019
a0e6cb4
Fix for part of comments by PeterNewman in PR1541.
Feb 26, 2019
0ba5dfd
Fix part of travis lint issues.
Feb 26, 2019
29b32d0
Added section about RDM support to README.
Feb 26, 2019
f451f19
Change for compiler error suggested by Peter, currently not next to i…
Feb 27, 2019
abdf4d5
Switch to using serial from FTDI device as serial part in RDM UID.
Feb 28, 2019
b080a84
Switched to using constant defined in header for Vendor part of UID.
Feb 28, 2019
9874f6d
Added logic to handle slow replies and make sure a full frame is rece…
Mar 3, 2019
ab0e961
Changed from OLA.* to *.extensions used by QtCreator at request of @P…
Mar 3, 2019
4b72f70
Fix embarrasing mistake with calculation inside sizeof()
Mar 3, 2019
17a40b1
Fix Travis lint issues.
Mar 4, 2019
532c8c3
Fix memory leak.
Mar 4, 2019
d28d3df
Fix more memory leaks.
Mar 6, 2019
92729c4
Fix for OS X building on Travis modified from PR1535
Mar 7, 2019
5f6187e
Further expanded README and hopefully shut up coverity.
Mar 7, 2019
1946005
Another attempt to appease the README markdown coverity complaints.
Mar 7, 2019
a4cfe30
Add another tested interface
peternewman Mar 9, 2019
faa98ca
Add that the Enttec Open DMX USB doesn't work
peternewman Mar 9, 2019
5b150c1
Minor tidy up and add details of more testing
peternewman Mar 9, 2019
fb51664
Merge branch 'ftdi-rdm' of https://github.com/Keeper-of-the-Keys/ola …
Mar 10, 2019
5564a1e
Initial test with nanosleep(2)
Mar 10, 2019
d533789
Added logic for non-branch broadcast packets that I had forgotten.
Mar 10, 2019
6309300
Fix lint issue
Mar 10, 2019
4553fe3
Extended StringToInt per request of @peternewman
Mar 10, 2019
591234c
Switched from use of strtoul() to ola::StringToInt()
Mar 10, 2019
3f60c66
Extended README with pinouts and schematics.
Mar 10, 2019
7d11c58
Enable Discovery-on-patch
Mar 10, 2019
f7489b6
Modify the description generator so that subsequent stuff doesn't blo…
Mar 10, 2019
c4d0ee3
Fix comments from @peternewman
Mar 10, 2019
c8e010f
Minor: remove backticks, they mess up display on github
Mar 10, 2019
7bf7ea5
Github display fix
Mar 10, 2019
6f84568
Changed to using only last 6 characters of serial.
Mar 10, 2019
f144552
Various comments by @peternewman fixed.
Mar 10, 2019
6b08435
Experimental replacement for usleep based on nanosleep.
Mar 11, 2019
b6d41af
Added ola::OlaSleep class based on the experimentalSleep stuff.
Mar 17, 2019
0a4a51e
Switched to using OlaSleep object, still need to remove native CheckG…
Mar 17, 2019
b084dcf
Added not about resistor strength and formatting changes to shut up c…
Mar 17, 2019
b489f96
README layout change to satisfy the coverity demon.
Mar 19, 2019
8d9691d
Rename OlaSleep to Sleep, modify & optimize some of the code.
Mar 19, 2019
1e004ac
Switch from Sleep::usleep() to this->usleep()
Mar 19, 2019
c222943
Added which output port the Interface deals with in Description()
Mar 19, 2019
8410da0
Make sure the Widget 'knows' the correct interface.
Mar 19, 2019
6252026
Lint fix
Mar 19, 2019
3c5a2e6
Detailed caller string for Sleep, some changes with granularity check…
Mar 19, 2019
4e285cb
Set a random deviceId for a device lacking a serial number in the thr…
Mar 20, 2019
c912c7f
Some doxygen changes
Mar 20, 2019
a90c1ba
Forgot to seed pseudo random number generator.
Mar 20, 2019
c640e4e
Merge remote-tracking branch 'upstream/master' into ftdi-rdm
Jun 26, 2019
8b9ff70
Added echo detection function, initially added at thread level howeve…
Jul 28, 2019
c2083af
Switch from srand to ola::math::Random()
Jul 29, 2019
717462d
Add LOG/DEBUG outputs to echo detection
Jul 29, 2019
7b0ecf8
Add logic to Write(ola::io::ByteString) to skip written bytes if echo…
Jul 29, 2019
ed1eb6b
Disable auto discovery.
Jul 29, 2019
5392ab0
Allow RDM to continue if no pending DMX packet and less then second p…
Jul 29, 2019
c666d71
Attempt at making Write(DmxBuffer) use Write(ola::io::ByteString) so
Jul 29, 2019
249d826
Fix description string to actually reflect what port is being used.
Jul 29, 2019
bfc14db
Now works regardless of echo state and biasing resistors.
Jul 29, 2019
48d1dbe
Updated docs to reflect state of driver.
Jul 29, 2019
a198e63
Fix some Travis spellintian issues
Jul 30, 2019
368a2f1
Merge remote-tracking branch 'upstream/master' into ftdi-rdm
Jul 30, 2019
e80f64d
Pad packets smaller then 24 slots to 24 slots, if forced to send out …
Aug 1, 2019
011f591
Fix spelling errors found by 'codespell' Travis-CI check.
Aug 1, 2019
b1dc145
Added FtdiDmxPluginDescription.h to lint test blacklist and moved sai…
Aug 2, 2019
a0d202a
lint file search expression fixed?
Aug 2, 2019
4566ff8
lint find statement fixed
Aug 2, 2019
c85f065
lint find statement
Aug 2, 2019
e48cebd
Another attempt for lint files
Aug 2, 2019
14a4513
fix wrong bracket in lint
Aug 2, 2019
117f831
Another attempt
Aug 2, 2019
1fb4bda
Merge branch 'master' into ftdi-rdm
peternewman Oct 9, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -246,3 +246,7 @@ javascript/new-src/node_modules
.cproject
.settings
.vscode/
*.config
*.creator*
*.files
*.includes
1 change: 1 addition & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,7 @@ before_install:
- if [[ "$TRAVIS_OS_NAME" == "osx" ]]; then brew reinstall libtool; fi
#Fix a broken homebrew python upgrade - see https://github.com/Homebrew/homebrew-core/issues/26358
- if [[ "$TRAVIS_OS_NAME" == "osx" ]]; then brew upgrade python || true; fi
- if [[ "$TRAVIS_OS_NAME" == "osx" ]]; then if [ ! -d /usr/local/sbin ]; then sudo mkdir -p /usr/local/sbin && sudo chown -R $(whoami) /usr/local/sbin; fi; fi
- if [[ "$TRAVIS_OS_NAME" == "osx" ]]; then brew install ccache bison flex liblo libmicrohttpd; fi # ossp-uuid, homebrew/python/numpy and libusb already present
- if [ "$TRAVIS_OS_NAME" == "osx" -a "$LIBFTDI" != "1" ]; then brew install libftdi0; fi # install libftdi0
- if [ "$TRAVIS_OS_NAME" == "osx" -a "$LIBFTDI" == "1" ]; then brew install libftdi; fi # install the latest libftdi
Expand Down
20 changes: 14 additions & 6 deletions common/utils/StringUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -152,13 +152,16 @@ bool StringToBoolTolerant(const string &value, bool *output) {
return false;
}

bool StringToInt(const string &value, unsigned int *output, bool strict) {
bool StringToInt(const string &value,
unsigned int *output,
bool strict,
uint8_t base) {
Copy link
Member

Choose a reason for hiding this comment

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

Please can we have some new tests for the base parameter.

Copy link
Member

Choose a reason for hiding this comment

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

Specifically, you're assuming, and the spec seems to imply, that it's case insensitive, but we should have a test of that so we catch issues on new platforms.

Copy link
Author

Choose a reason for hiding this comment

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

What do you mean by tests?
Whoever uses the function and actively defines a base surely knows the base of the string they are sending to it?

Copy link
Member

Choose a reason for hiding this comment

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

Like the stuff in https://github.com/OpenLightingProject/ola/blob/master/common/utils/StringUtilsTest.cpp#L541-L564 for example.

So passing in something in base 36, and also validating that 123ABC converts to the same value as 123abc.

if (value.empty()) {
return false;
}
char *end_ptr;
errno = 0;
long long l = strtoll(value.data(), &end_ptr, 10); // NOLINT(runtime/int)
long long l = strtoll(value.data(), &end_ptr, base); // NOLINT(runtime/int)
if (l < 0 || (l == 0 && errno != 0)) {
return false;
}
Expand All @@ -168,10 +171,12 @@ bool StringToInt(const string &value, unsigned int *output, bool strict) {
if (strict && *end_ptr != 0) {
return false;
}

*output = static_cast<unsigned int>(l);
Copy link
Member

Choose a reason for hiding this comment

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

This has no protection against truncating (which I think was deliberate for your use, but isn't good for the rest of the codebase)

Sorry, I meant can you add a long long version of StringToInt and use it for your particular case.

Then you can do that particular cast (or some sort of binary mask or whatever) in your specific code, rather than in the general function.

Copy link
Author

Choose a reason for hiding this comment

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

It's still returning false, a user who cares is anyhow checking for true/false.

Copy link
Member

Choose a reason for hiding this comment

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

But if we've returned false we shouldn't bother setting the value.


if (l > static_cast<long long>(UINT32_MAX)) { // NOLINT(runtime/int)
return false;
}
*output = static_cast<unsigned int>(l);
return true;
}

Expand Down Expand Up @@ -199,16 +204,19 @@ bool StringToInt(const string &value, uint8_t *output, bool strict) {
return true;
}

bool StringToInt(const string &value, int *output, bool strict) {
bool StringToInt(const string &value, int *output, bool strict, uint8_t base) {
if (value.empty()) {
return false;
}
char *end_ptr;
errno = 0;
long long l = strtoll(value.data(), &end_ptr, 10); // NOLINT(runtime/int)
long long l = strtoll(value.data(), &end_ptr, base); // NOLINT(runtime/int)
if (l == 0 && errno != 0) {
return false;
}

*output = static_cast<unsigned int>(l);

if (value == end_ptr) {
return false;
}
Expand All @@ -218,7 +226,7 @@ bool StringToInt(const string &value, int *output, bool strict) {
if (l < INT32_MIN || l > INT32_MAX) {
return false;
}
*output = static_cast<unsigned int>(l);

return true;
}

Expand Down
10 changes: 8 additions & 2 deletions include/ola/StringUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -262,12 +262,14 @@ bool StringToBoolTolerant(const std::string &value, bool *output);
* @param[in] value the string to convert
* @param[out] output a pointer where the value will be stored.
* @param[in] strict this controls if trailing characters produce an error.
* @param[in] base sets the base of the input string, default decimal (10)
* @returns true if the value was converted, false if the string was not an int
* or the value was too large / small for the type.
*/
bool StringToInt(const std::string &value,
unsigned int *output,
bool strict = false);
bool strict = false,
uint8_t base = 10);

/**
* @brief Convert a string to a uint16_t.
Expand Down Expand Up @@ -300,11 +302,15 @@ bool StringToInt(const std::string &value,
* @param[in] value the string to convert
* @param[out] output a pointer where the value will be stored.
* @param[in] strict this controls if trailing characters produce an error.
* @param[in] base sets the base of the input string, default decimal (10)
* @returns true if the value was converted, false if the string was not an int
* or the value was too large / small for the type.
* @sa StringToInt.
*/
bool StringToInt(const std::string &value, int *output, bool strict = false);
bool StringToInt(const std::string &value,
int *output,
bool strict = false,
uint8_t base = 10);

/**
* @brief Convert a string to a int16_t.
Expand Down
6 changes: 3 additions & 3 deletions plugins/convert_README_to_header.sh
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/bin/sh
#!/bin/bash

# A simple script to build a C++ header file containing the plugin description
# from the plugin's README.md
Expand Down Expand Up @@ -30,8 +30,8 @@ outfilename=`basename $outfile`;
# See http://stackoverflow.com/a/16576291
# On Mac OS's sed, \n is not recognized as a newline character, but
# \[actual newline] works
desc=`sed -e ':a' -e 'N' -e '$!ba' -e 's/\"/\\\"/g' -e 's/\n/\\\\n"\\
"/g' "$path/README.md"`;
desc=`sed -e ':a' -e 'N' -e '$!ba' -e 's#\\\#\\\\\\\#g' -e 's#\"#\\\"#g' -e 's#\n#\\\\n"\\
"#g' "$path/README.md"`;

identifier=`echo "PLUGINS_${plugin}_${outfilename%.h}_H_" | tr '[:lower:]' '[:upper:]'`

Expand Down
6 changes: 5 additions & 1 deletion plugins/ftdidmx/FtdiDmxDevice.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,19 @@ FtdiDmxDevice::~FtdiDmxDevice() {
bool FtdiDmxDevice::StartHook() {
unsigned int interface_count = m_widget->GetInterfaceCount();
unsigned int successfully_added = 0;
unsigned int serial = 0;

OLA_INFO << "Widget " << m_widget->Name() << " has " << interface_count
<< " interfaces.";

for (unsigned int i = 1; i <= interface_count; i++) {
if (!StringToInt(m_widget->Serial(), &serial, false, 36)) {
Copy link
Member

Choose a reason for hiding this comment

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

You're allowing trailing characters with strict=false, but if you buy two dongles at the same time, the leading ones are more likely to be the same, so truncating before passing to StringToInt would be more likely to generate unique UIDs.

Copy link
Author

Choose a reason for hiding this comment

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

You are right, I should just take the last 6 characters.

I probably also should now disallow dongles without a serial?

Copy link
Author

Choose a reason for hiding this comment

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

Did a change let me know what you think.

OLA_WARN << "StringToInt returned false, serial used: " << serial;
Copy link
Member

Choose a reason for hiding this comment

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

Don't you want to show the serial passed in, rather than the result which should be the untouched zero?

Copy link
Author

Choose a reason for hiding this comment

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

Well I changed the function that it does change the output regardless of it's error state, I figured that those who care will be checking if(StringToInt) and not if(output !=0) (which would we immensely stupid). But I'll grep through the code to make sure.

Copy link
Author

Choose a reason for hiding this comment

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

As far as I can tell all usages of StringToInt are inside if() statements.

}
FtdiInterface *port = new FtdiInterface(m_widget,
static_cast<ftdi_interface>(i));
if (port->SetupOutput()) {
AddPort(new FtdiDmxOutputPort(this, port, i, m_frequency));
AddPort(new FtdiDmxOutputPort(this, port, i, m_frequency, serial));
successfully_added += 1;
} else {
OLA_WARN << "Failed to add interface: " << i;
Expand Down
23 changes: 20 additions & 3 deletions plugins/ftdidmx/FtdiDmxPort.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,12 @@
#include <string>

#include "ola/DmxBuffer.h"
#include "ola/rdm/DiscoveryAgent.h"
#include "ola/rdm/RDMResponseCodes.h"

#include "olad/Port.h"
#include "olad/Preferences.h"

#include "plugins/ftdidmx/FtdiDmxDevice.h"
#include "plugins/ftdidmx/FtdiWidget.h"
#include "plugins/ftdidmx/FtdiDmxThread.h"
Expand All @@ -44,10 +48,11 @@ class FtdiDmxOutputPort : public ola::BasicOutputPort {
FtdiDmxOutputPort(FtdiDmxDevice *parent,
FtdiInterface *interface,
unsigned int id,
unsigned int freq)
: BasicOutputPort(parent, id),
unsigned int freq,
unsigned int serial)
: BasicOutputPort(parent, id, true, true),
m_interface(interface),
m_thread(interface, freq) {
m_thread(interface, freq, serial) {
m_thread.Start();
}
~FtdiDmxOutputPort() {
Expand All @@ -59,6 +64,18 @@ class FtdiDmxOutputPort : public ola::BasicOutputPort {
return m_thread.WriteDMX(buffer);
}

void SendRDMRequest(ola::rdm::RDMRequest *request,
ola::rdm::RDMCallback *callback) {
m_thread.SendRDMRequest(request, callback);
}

void RunFullDiscovery(ola::rdm::RDMDiscoveryCallback *callback) {
m_thread.RunFullDiscovery(callback);
}
void RunIncrementalDiscovery(ola::rdm::RDMDiscoveryCallback *callback) {
m_thread.RunIncrementalDiscovery(callback);
}

std::string Description() const { return m_interface->Description(); }

private:
Expand Down
Loading