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

WIP: Add support for RTL82328 dialect #35

Draft
wants to merge 10 commits into
base: realtek-poe
Choose a base branch
from

Conversation

mrnuke
Copy link
Collaborator

@mrnuke mrnuke commented Apr 25, 2024

THIS IS JUST A PROOF OF CONCEPT DO NOT NMERGE THIS!

This PR is for testing purposes only. Bu creating a PR, github should run a pipeline and create an installable package for others to test. That is the purpose of publishing a PR this early in development.

Add initial support for the PoE dialect used by RTL8238B PSE chips.

The original code separation assumed that there would be significant differences in the command structures. I designed the separation interface to be fairly high level, such as:

struct poe_dialect {
	int (*init_async)(const struct config *cfg);
	int (*init_ports_async)(const struct config *cfg);
	int (*enable_port_async)(uint8_t port, uint8_t enable);
	int (*poll_async)(const struct config *cfg);
	int (*handle_reply)(struct mcu_state *ctx, uint8_t *reply, size_t len);
};

Many commands are very similar between the the two dialects. The command IDs differ. The original split now menas tha a lot of code is duplicated. I am quite unhappy with the separation.

It is now clear that a better approach is to have common code implementing the dialects, and have a mapping bewteen command and command_id of the dialect.

enum poe_cmd {
	POE_CMD_NONE = 0,
	POE_CMD_SET_XYZ,
	...
	POE_CMD_SET_XYZ,
	...
};

poe_cmd_set_xyz(struct poe_dialect *dialect, ...)
{
	uint8_t cmd = dialect_map(dialect, POE_CMD_SET_XYZ);

	poe_cmd_queue(cmd, ...);
}

const struct dialect_map __map = {
	[POE_CMD_SET_XYZ] = { 0x42, CMD_4_PORT, CMD_ALL_PORT },
	...
};

The flags may seem obscure. Most command dealing with a port number can take more than one (port, value) argument pair. For example 01 <val>02 <val> 03 <val> ff ff, up to four pairs per command, with ff meaning "nothing to see here. This is the use case for the 4port command function:

static int poet_cmd_4_port(uint8_t cmd_id, uint8_t port[4], uint8_t data[4])
{
	uint8_t cmd[] = { cmd_id, 0x00, port[0], data[0], port[1], data[1],
		port[2], data[2], port[3], data[3] };

		return poe_cmd_queue(cmd, sizeof(cmd));
}

Other commands can use 7f as the port ID to configure all ports with the same value: 7f <val>. The flags in dialect map would indicate which commands can use such optimizations.

I am hoping that restructuring the code in this manner would reduce duplication, and actually make a PR that makes some sense.

Umh, this may prove to be a very bad idea.
This is wrong. A smart program will automatically detect the dialect,
including the baudrate. A buggy program requires the user to correctly
configure things. Being stupid takes no effort, so just do it!
The 8-port status command seems, once again, to be specific to
engenius devices. Out of about 30 query packets, the 8-port command
optimizes out one packet, and is specific to one vendor. That's genius!
@mrnuke mrnuke force-pushed the mrnuke-rtl8238b-devel branch from c9914a3 to 3e5071a Compare September 13, 2024 05:22
Apparently, some numbnut at github decided to make the actions
versioned. So although we have a perfectly working github pipeline, we
need to keep updating it whenever the github powers decide to
deprecate a number.

So update the "checkout" action from v2, to the diety-approved v4.
@mrnuke mrnuke force-pushed the mrnuke-rtl8238b-devel branch from c2ce0f1 to c49e44d Compare September 13, 2024 05:32
@Neustradamus
Copy link

To follow this PR :)

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 this pull request may close these issues.

Investigate feasibility of supporting RTL8238B-based protocol
2 participants