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

Personality example #29

Open
midikidi opened this issue Apr 4, 2021 · 6 comments · May be fixed by #39
Open

Personality example #29

midikidi opened this issue Apr 4, 2021 · 6 comments · May be fixed by #39

Comments

@midikidi
Copy link

midikidi commented Apr 4, 2021

This is the best RDM example I have found many thanks.I have ported DMXSerial2 to an STM32F103 and its working well. I am using the original code as a testbed(MEGA2560) to add RDM functionality. I am trying to add some personalities but I am not sure at what point I should send the Personality description.I am using a Swisson to debug RDM. I can set number of personalities to 10 and set the current personality and the Swisson shows the correct values. Any help would be greatly appreciated.

@peternewman
Copy link
Contributor

Hi @midikidi ,

The sensible thing would be to add it to the base library, so everyone can benefit (and not have to reinvent the wheel).

If RDMINIT was extended to take the RDMPERSONALITY, which is currently commented out, then adding a personalityDescription (32 chars) to each RDMPERSONALITY.

Then there's a GET DMX_PERSONALITY_DESCRIPTION:
http://rdm.openlighting.org/pid/display?manufacturer=0&pid=225

And GET and SET DMX_PERSONALITY:
http://rdm.openlighting.org/pid/display?manufacturer=0&pid=224

So you'd just need to add them to _processRDMMessage like some of the existing PIDs.

I'm biased, but you should test your code against the OLA RDM Responder Tests, the only (that I'm aware of) free and open source RDM test suite:
https://www.openlighting.org/rdm-tools/rdm-responder-tests/

You should definitely share your STM32F103 port, or possibly even do a PR to merge it as ifdef's if that's possible?

There's also this for the Teensy, which is adapted from bits of this code and others:
https://github.com/chrisstaite/TeensyDmx

Ideally everyone would just write separate low level libraries and there would be one unified RDM library on top, so everyone would get all the best features, but that doesn't seem to have happened yet unfortunately.

@midikidi
Copy link
Author

midikidi commented Apr 5, 2021

Hi Peter makes sense once sorted I will post the changes to DMXSerial2 and the port of STM32F103. The STM32 project is built with the STM32 Cube IDE not Arduino. I have added gets and sets to the parser but Im getting no where with the structures.

added to ino file loop()
//-----------------------------------------------------------------------------------
// ADDED 05.04.2021 E120_DMX_PERSONALITY DJH
//-----------------------------------------------------------------------------------
if (Parameter == SWAPINT(E120_DMX_PERSONALITY)) { // 0x00E0
if (CmdClass == E120_GET_COMMAND) {
Serial.println(" DEBUG E120_GET_COMMAND+E120_DMX_PERSONALITY");
if (rdm->DataLength > 0) {
// Unexpected data
*nackReason = E120_NR_FORMAT_ERROR;
}
else if (rdm->SubDev != 0) {
// No sub-devices supported
*nackReason = E120_NR_SUB_DEVICE_OUT_OF_RANGE;
}
else {
rdm->DataLength = 1;
rdm->Data[0] = currentpersonality;

				handled = true;
			}
		}
		else if (CmdClass == E120_SET_COMMAND) {
			// This device doesn't support set
			*nackReason = E120_NR_UNSUPPORTED_COMMAND_CLASS;
		}

	}

	else if (Parameter == SWAPINT(E120_DMX_PERSONALITY)) { // 0x00E0
		if (CmdClass == E120_SET_COMMAND) {

			Serial.println(" DEBUG E120_SET_COMMAND+E120_DMX_PERSONALITY");

			if (rdm->DataLength > 0) {
				// Oversized data
				*nackReason = E120_NR_FORMAT_ERROR;
			}
			else {
				uint8_t newPersonalityNumber = rdm->Data[0];

				currentpersonality = newPersonalityNumber;
				rdm->DataLength = 0;
				// persist in EEPROM
				//need to add _NewPersonality to eeprom save
				//_saveEEPRom();
				handled = true;
			}
		}

		else if ((CmdClass == E120_GET_COMMAND) && (Parameter == SWAPINT(E120_DMX_PERSONALITY_DESCRIPTION))) { // 0x00E1
			Serial.println(" DEBUG E120_GET_COMMAND+E120_DMX_PERSONALITY_DESCRIPTION");
			if (rdm->DataLength > 0) {
				// Unexpected data
				*nackReason = E120_NR_FORMAT_ERROR;
			}
			else if (rdm->SubDev != 0) {
				// No sub-devices supported
				*nackReason = E120_NR_SUB_DEVICE_OUT_OF_RANGE;
			}
			else {
				// return the PERSONALITY_LABEL
				rdm->DataLength = strlen(_personalityLabel);
				memcpy(rdm->Data, _personalityLabel, rdm->DataLength);
				handled = true;
			}
		}
	}

//--------------------------------------------------------------------------------------
struct RDMINIT {
const char *manufacturerLabel;
const uint16_t deviceModelId;
const char *deviceModel;
uint16_t footprint;
//uint16_t personalityCount;
const RDMPERSONALITY *personalities;
const uint16_t additionalCommandsLength;
const uint16_t *additionalCommands;
const uint8_t sensorsLength;
const RDMSENSOR *sensors;
}; // struct RDMINIT

struct RDMPERSONALITY {

uint8_t personalityCount=5;
uint8_t currentpersonality=1;
char personalitylabel[32]="personality1";
// maybe more here... when supporting more personalitites.
}; // struct RDMPERSONALITY

//error here <cannot be initalised as const int
struct RDMINIT rdmInit = {
"MyLight", // Manufacturer Label
1, // Device Model ID
"PWM RDM Device", // Device Model Label
3, // footprint
(sizeof(my_pids) / sizeof(uint16_t)), my_pids,
0, NULL
};

Thankyou

@peternewman
Copy link
Contributor

Hi Peter makes sense once sorted I will post the changes to DMXSerial2 and the port of STM32F103. The STM32 project is built with the STM32 Cube IDE not Arduino. I have added gets and sets to the parser but Im getting no where with the structures.

Please post it as a PR, it means people can make comments in-line on your code which makes reviewing and commenting SO much easier and it will all be properly formatted (which some of it currently isn't in the comment), plus it will run any continuous integration tests set up in the repo. Plus someone can pull it or try making changes knowing they are running the same code as you.

The initial bit, you want to format it like this within RDMINIT:

uint16_t personalityCount; // Total number of personalities
const RDMPERSONALITY *personalities; // Array of RDMPERSONALITY

Then:

struct RDMPERSONALITY {
uint16_t footprint; // Number of channels the personality takes
char personalitylabel[32]="personality1";
// maybe more here... when supporting more personalitites.
}; // struct RDMPERSONALITY

Take these bits out:

uint8_t personalityCount=5; // Stored in RDMINIT
uint8_t currentpersonality=1; // Stored internally, you may want to extend init so the initial personality can be set, or maybe it's always the first or last one.

@midikidi
Copy link
Author

midikidi commented Apr 6, 2021

Hi Pete, Im completely new to Github .I have added the zipped folder of the original code of DMXSerial2 with my changes added get personality description / get-set personality number. I changed RDMINIT struct and populated RDMPERSONALITY.there are compilation errors to do with the structure changed.
DMXSerial2Personality.zip.
Re the STM32 version for Cube IDE. should I just send you the zipped folder

@peternewman
Copy link
Contributor

Hi Pete, Im completely new to Github .

That's okay, we all have to start somewhere? Have you used Git before? What OS are you using? They have some great guides here:
https://docs.github.com/en

I'd probably start with this:
https://docs.github.com/en/github/getting-started-with-github/fork-a-repo

And:
https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/creating-a-pull-request-from-a-fork

They've even got a desktop app:
https://docs.github.com/en/desktop/contributing-and-collaborating-using-github-desktop

Or you can copy all your edits into the files on the website.

I have added the zipped folder of the original code of DMXSerial2 with my changes added get personality description / get-set personality number.

That's a good start, but it's still no where near as easy to comment on as a pull request unfortunately. I don't personally have the time to extract your zip files and diff the results, but I can spare some time to view a PR and comment on what might be going wrong.

I changed RDMINIT struct and populated RDMPERSONALITY.there are compilation errors to do with the structure changed.

Sharing the compilation errors is always a good start.

Re the STM32 version for Cube IDE. should I just send you the zipped folder

I'm not personally particularly interested in an STM32 version (I'm also not the author of this library, although I have contributed fairly heavily to it at times). I'm more interested in people generally producing good RDM implementations and avoiding too much reinventing the wheel usually helps that. I see the base DMXSerial2.cpp includes Arduino.h, which presumably you have to substitute with something else? Can those things be covered with ifdef macros ( https://www.deviceplus.com/arduino/arduino-preprocessor-directives-tutorial/ )? Then your STM32 code could just use the main library and so you wouldn't have to keep porting changes and improvements from one to the other. Hopefully @mathertel would be amenable to making the library a bit more cross-platform if it's a fairly easy fix?

@peternewman peternewman linked a pull request Jul 14, 2021 that will close this issue
@peternewman
Copy link
Contributor

I've got personalities working and passing all the OLA RDM tests here #39 .

This currently needs some changes to the data passed into the RDMINIT struct, but they're fairly self-explanatory.

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 a pull request may close this issue.

2 participants