Thread: Putting char in unsigned char

Well, that helped.. With that line you've just sent I've realised something quite logical... Oops.

This line from post #7:

Code:

 unsigned intresult = 100 * (packetBuffer[0] - '0') + 10 * (packetBuffer[1] - '0') + (packetBuffer - '0');

shoud be:

Code:

unsigned intresult = 100 * (packetBuffer[0] - '0') + 10 * (packetBuffer[1] - '0') + (packetBuffer[2] - '0');

Like this the ID is set correctly.
I still got one weird answer from the device like mentioned in post #11 at point 6

The interesting thing is that the device will always respond: (current)ID + " : This command is not recognized"

The ID is always 225, no matter which value I set as ID.

I still have this issue, but the ID is now always 2 instead of 225.

Also, facing this code; what is the minimum possible value of ID and what is the maximum possible value of ID?

The code you mentioned in post #13, I should delete that... It's not necessary

Originally Posted by laserlight

I wrote "where the type of numBytes is the return type of Udp.read". This has absolutely nothing to do with the type of packetBuffer, which is in fact char[256], not char. The return type of Udp.read is probably size_t, or another unsigned integer type, or possibly a signed integer type (most probably int or larger, in that case). This makes sense: packetBuffer is an array of char because it is well, a buffer, i.e., the bytes read themselves, whereas the return type of Udp.read is some type suited to representing the number of bytes read.


That's what I wrote in post #6: "You can also do additional error checking to confirm that the content of packetBuffer are digits before you do the computation." You don't have to do this error checking, but then if the buffer does have three bytes read yet is invalid, your users will either get harder to understand error messages, or worse still, an incorrect tableID that could be very hard to diagnose.


That's what Click_here told you in post #8 and which you said worked in post #9, except that the ID was not set correctly. Admittedly it was a typo error on my part, but I was typing it out on my phone so it is more difficult to spot such mistakes.


You might want to post your current code or update your pastebin.

The reason why you have 861 lines of code in your pastebin right now is that you haven't made good use of abstraction: you keep repeating the same code over and over again instead of moving them into functions that you can call with varying arguments. This is sometimes called the DRY principle: Don't Repeat Yourself.

I also understand that you might reasonably have global state and hence global variables, but you should still think very carefully before you declare a global variable. Just because a variable is used in multiple functions doesn't mean it represents global state, e.g., lightSensor is always used locally to store the return value of a function, so it should be a local variable. You should also use const for variables that actually represent constants rather than variables to be modified at run time.


That's something that's domain specific, i.e., you would need to refer to the documentation for this UDP multicolor lamp. What I can tell you is that by storing the ID in an unsigned char, you're limited to the range [0, UCHAR_MAX], where UCHAR_MAX is at least 255.

Thanks laserlight! Will dive into this tomorrow, to enhance my code.

I think what's partially responsible for the confusion is the incorrect and confusing documentation for UDP.read():

Returns
char : returns the characters in the buffer

I wasn't sure what that meant since the first argument, packetBuffer, holds the data in the buffer, so why would it also need to return the data (or "characters") in the buffer? Then I looked at the header file for UDP which gave far more insight into its use in just three lines than the documentation does:

Code:

    // Read up to len bytes from the current packet and place them into buffer
    // Returns the number of bytes read, or 0 if none are available
    virtual int read(unsigned char* buffer, size_t len) =0;

So it actually returns an int which is the number of bytes read.

Originally Posted by laserlight

That looks like a bug in the documentation

I have updated my pastebin, not big changes yet but I did some cleaning.
I'll change also some of my booleans and replace them with enums.
I did already add them, since I'm also working for a fade between the light modes.. terraslamp - Pastebin.com

Would be nice if we still could fix that '2' ID somehow, if that is fixed, then I have a solid and real cool lamp.
That line starts on 725

While I'm here, I do have one more issue, I'll just mention it, but if I'm going to far with all my questions, then I'll agree to that.
I like this forum the most in similar with for example the Arduino Forum.. So I'll just give it a go


As you have noticed I have a light sensor(LDR), that detects whether it's day or night. The lamp can turn then automatically on and automatically off.
What I'm trying to achieve, is that whenever the LDR measures a value that triggers the other mode, it should first check that value for 10 minutes (or similar), to make sure it actually is day/night.
Do notice, in my code it's 30seconds for debug.

For this I use a function started on line 648(night) and line 668 (day).
The millis() function does not work as supposed, which does makes sense.
As soon as the lightmode changes, it will start counting, after 30s I can switch directly to the other mode..
While what I want is that millis starts counting, from the moment that the sensor detects a value within the range of the other mode.