Fix zone parsing desync on OpenRGB 1.0rc3+ servers- #1

[septicwolf818]

Servers with SDK protocol >= 5 serialize a matrix map block for every zone, not just MATRIX zones. ZoneData.unpack only consumed it when zone_type == MATRIX, leaving the parser 8 bytes behind on LINEAR/SINGLE zones and producing garbage values (e.g. "256 is not a valid ZoneType") during device discovery.

Consume the matrix block whenever matrix_zone_size > 0, which is backward compatible since the size is 0 when no map exists. Also widen ZoneType to the current server values (LINEAR_LOOP, MATRIX_LOOP_X, MATRIX_LOOP_Y, SEGMENTED) and add a missing fallback so future types don't crash.

[CalcProgrammer1]

Is this only on pipeline? If so it's a bug and needs fixed OpenRGB side, the 5 and earlier protocols should not be changed. If backwards compatibility broke we need to fix it.

[CalcProgrammer1]

Looking at the older OpenRGB code, it actually sends matrix map only if a matrix map exists, not necessarily if type is matrix. It does so happen that 99 if not 100% of controllers that had a non-null matrix map pointer also had ZONE_TYPE_MATRIX, but the old implementation of the server didn't necessarily guarantee this relationship either so in this case I'd say the parser here is in the wrong.

For instance, in the 0.8 release's client side parser, you can see it checks the matrix map size to determine whether or not to parse a matrix map, not the type value:

https://gitlab.com/CalcProgrammer1/OpenRGB/-/blob/release_0.8/RGBController/RGBController.cpp?ref_type=tags#L742

On OpenRGB next (the code that was merged after 1.0rc3 and will soon be released as 1.0), matrix maps are no longer handled as pointers, so every zone "has" a matrix map. I could update the OpenRGB side to not send empty matrix maps (w 0, h 0, map size 0) which would match the old behavior better, save bytes on the network, and reduce breakage caused by this issue. I think I will do this, but this issue should still be fixed here as it was working by happenstance as is.

[septicwolf818]

@CalcProgrammer1 Yes, it's only on next (the post-1.0rc3 pipeline). In release_0.8 and earlier, matrix_map is a pointer and the block is only written when it's non-NULL, regardless of zone type - so 5-and-earlier are untouched there. On next, matrix_map became a value member (matrix_map_type matrix_map, RGBControllerInterface.h:427), and GetMatrixMapDescriptionSize (RGBController.cpp:2792) isn't gated by protocol version, so a block is now sent for every zone at any negotiated protocol version (0-5 included). That does change the wire data for the older protocols when talking to a next server - so yes, it's a next-side backwards-compat break. Skipping empty maps (size 0 when h==0 && w==0) on next would restore the old behavior. I have aligned the python parser to key off the size field like the 0.8 client, so it works with both.

[CalcProgrammer1]

Pushed a fix on the OpenRGB side to not send empty maps, but this change still should be implemented on the Python side to correctly match OpenRGB's own parser.

[jath03]

Interesting. I wasn't aware that other zone types could have a matrix map

[CalcProgrammer1]

Interesting. I wasn't aware that other zone types could have a matrix map

In previous versions of OpenRGB the protocol did not exclude the possibility, but I don't know of any controllers that actually did it. On 1.0, I've expanded manually configurable zones (ARGB zones) such that the user has control over the Type field. They can set a matrix map and set the Type field as two independent user options. The matrix map is saved no matter what the type field is set to which allows toggling between matrix and non matrix layouts easily. That is the main place you will see this behavior in practice now.

[CalcProgrammer1]

It also now allows device controllers to build in their own matrix map but make it optional, a keyboard could have an underglow zone that could be switched between mapping into the keyboard matrix or working as an independent light ring.

[septicwolf818]

Servers with SDK protocol >= 5 serialize a matrix map block for every zone, not just MATRIX zones. ZoneData.unpack only consumed it when zone_type == MATRIX, leaving the parser 8 bytes behind on LINEAR/SINGLE zones and producing garbage values (e.g. "256 is not a valid ZoneType") during device discovery.

Consume the matrix block whenever matrix_zone_size > 0, which is backward compatible since the size is 0 when no map exists. Also widen ZoneType to the current server values (LINEAR_LOOP, MATRIX_LOOP_X, MATRIX_LOOP_Y, SEGMENTED) and add a missing fallback so future types don't crash.

[CalcProgrammer1]

Is this only on pipeline? If so it's a bug and needs fixed OpenRGB side, the 5 and earlier protocols should not be changed. If backwards compatibility broke we need to fix it.

[CalcProgrammer1]

Looking at the older OpenRGB code, it actually sends matrix map only if a matrix map exists, not necessarily if type is matrix. It does so happen that 99 if not 100% of controllers that had a non-null matrix map pointer also had ZONE_TYPE_MATRIX, but the old implementation of the server didn't necessarily guarantee this relationship either so in this case I'd say the parser here is in the wrong.

For instance, in the 0.8 release's client side parser, you can see it checks the matrix map size to determine whether or not to parse a matrix map, not the type value:

https://gitlab.com/CalcProgrammer1/OpenRGB/-/blob/release_0.8/RGBController/RGBController.cpp?ref_type=tags#L742

On OpenRGB next (the code that was merged after 1.0rc3 and will soon be released as 1.0), matrix maps are no longer handled as pointers, so every zone "has" a matrix map. I could update the OpenRGB side to not send empty matrix maps (w 0, h 0, map size 0) which would match the old behavior better, save bytes on the network, and reduce breakage caused by this issue. I think I will do this, but this issue should still be fixed here as it was working by happenstance as is.

[septicwolf818]

@CalcProgrammer1 Yes, it's only on next (the post-1.0rc3 pipeline). In release_0.8 and earlier, matrix_map is a pointer and the block is only written when it's non-NULL, regardless of zone type - so 5-and-earlier are untouched there. On next, matrix_map became a value member (matrix_map_type matrix_map, RGBControllerInterface.h:427), and GetMatrixMapDescriptionSize (RGBController.cpp:2792) isn't gated by protocol version, so a block is now sent for every zone at any negotiated protocol version (0-5 included). That does change the wire data for the older protocols when talking to a next server - so yes, it's a next-side backwards-compat break. Skipping empty maps (size 0 when h==0 && w==0) on next would restore the old behavior. I have aligned the python parser to key off the size field like the 0.8 client, so it works with both.

[CalcProgrammer1]

Pushed a fix on the OpenRGB side to not send empty maps, but this change still should be implemented on the Python side to correctly match OpenRGB's own parser.

[jath03]

Interesting. I wasn't aware that other zone types could have a matrix map

[CalcProgrammer1]

Interesting. I wasn't aware that other zone types could have a matrix map

In previous versions of OpenRGB the protocol did not exclude the possibility, but I don't know of any controllers that actually did it. On 1.0, I've expanded manually configurable zones (ARGB zones) such that the user has control over the Type field. They can set a matrix map and set the Type field as two independent user options. The matrix map is saved no matter what the type field is set to which allows toggling between matrix and non matrix layouts easily. That is the main place you will see this behavior in practice now.

[CalcProgrammer1]

It also now allows device controllers to build in their own matrix map but make it optional, a keyboard could have an underglow zone that could be switched between mapping into the keyboard matrix or working as an independent light ring.

[septicwolf818]

Servers with SDK protocol >= 5 serialize a matrix map block for every zone, not just MATRIX zones. ZoneData.unpack only consumed it when zone_type == MATRIX, leaving the parser 8 bytes behind on LINEAR/SINGLE zones and producing garbage values (e.g. "256 is not a valid ZoneType") during device discovery.

Consume the matrix block whenever matrix_zone_size > 0, which is backward compatible since the size is 0 when no map exists. Also widen ZoneType to the current server values (LINEAR_LOOP, MATRIX_LOOP_X, MATRIX_LOOP_Y, SEGMENTED) and add a missing fallback so future types don't crash.

[CalcProgrammer1]

Is this only on pipeline? If so it's a bug and needs fixed OpenRGB side, the 5 and earlier protocols should not be changed. If backwards compatibility broke we need to fix it.

[CalcProgrammer1]

Looking at the older OpenRGB code, it actually sends matrix map only if a matrix map exists, not necessarily if type is matrix. It does so happen that 99 if not 100% of controllers that had a non-null matrix map pointer also had ZONE_TYPE_MATRIX, but the old implementation of the server didn't necessarily guarantee this relationship either so in this case I'd say the parser here is in the wrong.

For instance, in the 0.8 release's client side parser, you can see it checks the matrix map size to determine whether or not to parse a matrix map, not the type value:

https://gitlab.com/CalcProgrammer1/OpenRGB/-/blob/release_0.8/RGBController/RGBController.cpp?ref_type=tags#L742

On OpenRGB next (the code that was merged after 1.0rc3 and will soon be released as 1.0), matrix maps are no longer handled as pointers, so every zone "has" a matrix map. I could update the OpenRGB side to not send empty matrix maps (w 0, h 0, map size 0) which would match the old behavior better, save bytes on the network, and reduce breakage caused by this issue. I think I will do this, but this issue should still be fixed here as it was working by happenstance as is.

[septicwolf818]

@CalcProgrammer1 Yes, it's only on next (the post-1.0rc3 pipeline). In release_0.8 and earlier, matrix_map is a pointer and the block is only written when it's non-NULL, regardless of zone type - so 5-and-earlier are untouched there. On next, matrix_map became a value member (matrix_map_type matrix_map, RGBControllerInterface.h:427), and GetMatrixMapDescriptionSize (RGBController.cpp:2792) isn't gated by protocol version, so a block is now sent for every zone at any negotiated protocol version (0-5 included). That does change the wire data for the older protocols when talking to a next server - so yes, it's a next-side backwards-compat break. Skipping empty maps (size 0 when h==0 && w==0) on next would restore the old behavior. I have aligned the python parser to key off the size field like the 0.8 client, so it works with both.

[CalcProgrammer1]

Pushed a fix on the OpenRGB side to not send empty maps, but this change still should be implemented on the Python side to correctly match OpenRGB's own parser.

[jath03]

Interesting. I wasn't aware that other zone types could have a matrix map

[CalcProgrammer1]

Interesting. I wasn't aware that other zone types could have a matrix map

In previous versions of OpenRGB the protocol did not exclude the possibility, but I don't know of any controllers that actually did it. On 1.0, I've expanded manually configurable zones (ARGB zones) such that the user has control over the Type field. They can set a matrix map and set the Type field as two independent user options. The matrix map is saved no matter what the type field is set to which allows toggling between matrix and non matrix layouts easily. That is the main place you will see this behavior in practice now.

[CalcProgrammer1]

It also now allows device controllers to build in their own matrix map but make it optional, a keyboard could have an underglow zone that could be switched between mapping into the keyboard matrix or working as an independent light ring.

[septicwolf818]

Servers with SDK protocol >= 5 serialize a matrix map block for every zone, not just MATRIX zones. ZoneData.unpack only consumed it when zone_type == MATRIX, leaving the parser 8 bytes behind on LINEAR/SINGLE zones and producing garbage values (e.g. "256 is not a valid ZoneType") during device discovery.

Consume the matrix block whenever matrix_zone_size > 0, which is backward compatible since the size is 0 when no map exists. Also widen ZoneType to the current server values (LINEAR_LOOP, MATRIX_LOOP_X, MATRIX_LOOP_Y, SEGMENTED) and add a missing fallback so future types don't crash.

[CalcProgrammer1]

Is this only on pipeline? If so it's a bug and needs fixed OpenRGB side, the 5 and earlier protocols should not be changed. If backwards compatibility broke we need to fix it.

[CalcProgrammer1]

Looking at the older OpenRGB code, it actually sends matrix map only if a matrix map exists, not necessarily if type is matrix. It does so happen that 99 if not 100% of controllers that had a non-null matrix map pointer also had ZONE_TYPE_MATRIX, but the old implementation of the server didn't necessarily guarantee this relationship either so in this case I'd say the parser here is in the wrong.

For instance, in the 0.8 release's client side parser, you can see it checks the matrix map size to determine whether or not to parse a matrix map, not the type value:

https://gitlab.com/CalcProgrammer1/OpenRGB/-/blob/release_0.8/RGBController/RGBController.cpp?ref_type=tags#L742

On OpenRGB next (the code that was merged after 1.0rc3 and will soon be released as 1.0), matrix maps are no longer handled as pointers, so every zone "has" a matrix map. I could update the OpenRGB side to not send empty matrix maps (w 0, h 0, map size 0) which would match the old behavior better, save bytes on the network, and reduce breakage caused by this issue. I think I will do this, but this issue should still be fixed here as it was working by happenstance as is.

[septicwolf818]

@CalcProgrammer1 Yes, it's only on next (the post-1.0rc3 pipeline). In release_0.8 and earlier, matrix_map is a pointer and the block is only written when it's non-NULL, regardless of zone type - so 5-and-earlier are untouched there. On next, matrix_map became a value member (matrix_map_type matrix_map, RGBControllerInterface.h:427), and GetMatrixMapDescriptionSize (RGBController.cpp:2792) isn't gated by protocol version, so a block is now sent for every zone at any negotiated protocol version (0-5 included). That does change the wire data for the older protocols when talking to a next server - so yes, it's a next-side backwards-compat break. Skipping empty maps (size 0 when h==0 && w==0) on next would restore the old behavior. I have aligned the python parser to key off the size field like the 0.8 client, so it works with both.

[CalcProgrammer1]

Pushed a fix on the OpenRGB side to not send empty maps, but this change still should be implemented on the Python side to correctly match OpenRGB's own parser.

[jath03]

Interesting. I wasn't aware that other zone types could have a matrix map

[CalcProgrammer1]

Interesting. I wasn't aware that other zone types could have a matrix map

In previous versions of OpenRGB the protocol did not exclude the possibility, but I don't know of any controllers that actually did it. On 1.0, I've expanded manually configurable zones (ARGB zones) such that the user has control over the Type field. They can set a matrix map and set the Type field as two independent user options. The matrix map is saved no matter what the type field is set to which allows toggling between matrix and non matrix layouts easily. That is the main place you will see this behavior in practice now.

[CalcProgrammer1]

It also now allows device controllers to build in their own matrix map but make it optional, a keyboard could have an underglow zone that could be switched between mapping into the keyboard matrix or working as an independent light ring.

[septicwolf818]

Servers with SDK protocol >= 5 serialize a matrix map block for every zone, not just MATRIX zones. ZoneData.unpack only consumed it when zone_type == MATRIX, leaving the parser 8 bytes behind on LINEAR/SINGLE zones and producing garbage values (e.g. "256 is not a valid ZoneType") during device discovery.

Consume the matrix block whenever matrix_zone_size > 0, which is backward compatible since the size is 0 when no map exists. Also widen ZoneType to the current server values (LINEAR_LOOP, MATRIX_LOOP_X, MATRIX_LOOP_Y, SEGMENTED) and add a missing fallback so future types don't crash.

[CalcProgrammer1]

Is this only on pipeline? If so it's a bug and needs fixed OpenRGB side, the 5 and earlier protocols should not be changed. If backwards compatibility broke we need to fix it.

[CalcProgrammer1]

Looking at the older OpenRGB code, it actually sends matrix map only if a matrix map exists, not necessarily if type is matrix. It does so happen that 99 if not 100% of controllers that had a non-null matrix map pointer also had ZONE_TYPE_MATRIX, but the old implementation of the server didn't necessarily guarantee this relationship either so in this case I'd say the parser here is in the wrong.

For instance, in the 0.8 release's client side parser, you can see it checks the matrix map size to determine whether or not to parse a matrix map, not the type value:

https://gitlab.com/CalcProgrammer1/OpenRGB/-/blob/release_0.8/RGBController/RGBController.cpp?ref_type=tags#L742

On OpenRGB next (the code that was merged after 1.0rc3 and will soon be released as 1.0), matrix maps are no longer handled as pointers, so every zone "has" a matrix map. I could update the OpenRGB side to not send empty matrix maps (w 0, h 0, map size 0) which would match the old behavior better, save bytes on the network, and reduce breakage caused by this issue. I think I will do this, but this issue should still be fixed here as it was working by happenstance as is.

[septicwolf818]

@CalcProgrammer1 Yes, it's only on next (the post-1.0rc3 pipeline). In release_0.8 and earlier, matrix_map is a pointer and the block is only written when it's non-NULL, regardless of zone type - so 5-and-earlier are untouched there. On next, matrix_map became a value member (matrix_map_type matrix_map, RGBControllerInterface.h:427), and GetMatrixMapDescriptionSize (RGBController.cpp:2792) isn't gated by protocol version, so a block is now sent for every zone at any negotiated protocol version (0-5 included). That does change the wire data for the older protocols when talking to a next server - so yes, it's a next-side backwards-compat break. Skipping empty maps (size 0 when h==0 && w==0) on next would restore the old behavior. I have aligned the python parser to key off the size field like the 0.8 client, so it works with both.

[CalcProgrammer1]

Pushed a fix on the OpenRGB side to not send empty maps, but this change still should be implemented on the Python side to correctly match OpenRGB's own parser.

[jath03]

Interesting. I wasn't aware that other zone types could have a matrix map

[CalcProgrammer1]

Interesting. I wasn't aware that other zone types could have a matrix map

In previous versions of OpenRGB the protocol did not exclude the possibility, but I don't know of any controllers that actually did it. On 1.0, I've expanded manually configurable zones (ARGB zones) such that the user has control over the Type field. They can set a matrix map and set the Type field as two independent user options. The matrix map is saved no matter what the type field is set to which allows toggling between matrix and non matrix layouts easily. That is the main place you will see this behavior in practice now.

[CalcProgrammer1]

It also now allows device controllers to build in their own matrix map but make it optional, a keyboard could have an underglow zone that could be switched between mapping into the keyboard matrix or working as an independent light ring.

[septicwolf818]

Servers with SDK protocol >= 5 serialize a matrix map block for every zone, not just MATRIX zones. ZoneData.unpack only consumed it when zone_type == MATRIX, leaving the parser 8 bytes behind on LINEAR/SINGLE zones and producing garbage values (e.g. "256 is not a valid ZoneType") during device discovery.

Consume the matrix block whenever matrix_zone_size > 0, which is backward compatible since the size is 0 when no map exists. Also widen ZoneType to the current server values (LINEAR_LOOP, MATRIX_LOOP_X, MATRIX_LOOP_Y, SEGMENTED) and add a missing fallback so future types don't crash.

[CalcProgrammer1]

Is this only on pipeline? If so it's a bug and needs fixed OpenRGB side, the 5 and earlier protocols should not be changed. If backwards compatibility broke we need to fix it.

[CalcProgrammer1]

Looking at the older OpenRGB code, it actually sends matrix map only if a matrix map exists, not necessarily if type is matrix. It does so happen that 99 if not 100% of controllers that had a non-null matrix map pointer also had ZONE_TYPE_MATRIX, but the old implementation of the server didn't necessarily guarantee this relationship either so in this case I'd say the parser here is in the wrong.

For instance, in the 0.8 release's client side parser, you can see it checks the matrix map size to determine whether or not to parse a matrix map, not the type value:

https://gitlab.com/CalcProgrammer1/OpenRGB/-/blob/release_0.8/RGBController/RGBController.cpp?ref_type=tags#L742

On OpenRGB next (the code that was merged after 1.0rc3 and will soon be released as 1.0), matrix maps are no longer handled as pointers, so every zone "has" a matrix map. I could update the OpenRGB side to not send empty matrix maps (w 0, h 0, map size 0) which would match the old behavior better, save bytes on the network, and reduce breakage caused by this issue. I think I will do this, but this issue should still be fixed here as it was working by happenstance as is.

[septicwolf818]

@CalcProgrammer1 Yes, it's only on next (the post-1.0rc3 pipeline). In release_0.8 and earlier, matrix_map is a pointer and the block is only written when it's non-NULL, regardless of zone type - so 5-and-earlier are untouched there. On next, matrix_map became a value member (matrix_map_type matrix_map, RGBControllerInterface.h:427), and GetMatrixMapDescriptionSize (RGBController.cpp:2792) isn't gated by protocol version, so a block is now sent for every zone at any negotiated protocol version (0-5 included). That does change the wire data for the older protocols when talking to a next server - so yes, it's a next-side backwards-compat break. Skipping empty maps (size 0 when h==0 && w==0) on next would restore the old behavior. I have aligned the python parser to key off the size field like the 0.8 client, so it works with both.

[CalcProgrammer1]

Pushed a fix on the OpenRGB side to not send empty maps, but this change still should be implemented on the Python side to correctly match OpenRGB's own parser.

[jath03]

Interesting. I wasn't aware that other zone types could have a matrix map

[CalcProgrammer1]

Interesting. I wasn't aware that other zone types could have a matrix map

In previous versions of OpenRGB the protocol did not exclude the possibility, but I don't know of any controllers that actually did it. On 1.0, I've expanded manually configurable zones (ARGB zones) such that the user has control over the Type field. They can set a matrix map and set the Type field as two independent user options. The matrix map is saved no matter what the type field is set to which allows toggling between matrix and non matrix layouts easily. That is the main place you will see this behavior in practice now.

[CalcProgrammer1]

It also now allows device controllers to build in their own matrix map but make it optional, a keyboard could have an underglow zone that could be switched between mapping into the keyboard matrix or working as an independent light ring.

[septicwolf818]

Servers with SDK protocol >= 5 serialize a matrix map block for every zone, not just MATRIX zones. ZoneData.unpack only consumed it when zone_type == MATRIX, leaving the parser 8 bytes behind on LINEAR/SINGLE zones and producing garbage values (e.g. "256 is not a valid ZoneType") during device discovery.

Consume the matrix block whenever matrix_zone_size > 0, which is backward compatible since the size is 0 when no map exists. Also widen ZoneType to the current server values (LINEAR_LOOP, MATRIX_LOOP_X, MATRIX_LOOP_Y, SEGMENTED) and add a missing fallback so future types don't crash.

[CalcProgrammer1]

Is this only on pipeline? If so it's a bug and needs fixed OpenRGB side, the 5 and earlier protocols should not be changed. If backwards compatibility broke we need to fix it.

[CalcProgrammer1]

Looking at the older OpenRGB code, it actually sends matrix map only if a matrix map exists, not necessarily if type is matrix. It does so happen that 99 if not 100% of controllers that had a non-null matrix map pointer also had ZONE_TYPE_MATRIX, but the old implementation of the server didn't necessarily guarantee this relationship either so in this case I'd say the parser here is in the wrong.

For instance, in the 0.8 release's client side parser, you can see it checks the matrix map size to determine whether or not to parse a matrix map, not the type value:

https://gitlab.com/CalcProgrammer1/OpenRGB/-/blob/release_0.8/RGBController/RGBController.cpp?ref_type=tags#L742

On OpenRGB next (the code that was merged after 1.0rc3 and will soon be released as 1.0), matrix maps are no longer handled as pointers, so every zone "has" a matrix map. I could update the OpenRGB side to not send empty matrix maps (w 0, h 0, map size 0) which would match the old behavior better, save bytes on the network, and reduce breakage caused by this issue. I think I will do this, but this issue should still be fixed here as it was working by happenstance as is.

[septicwolf818]

@CalcProgrammer1 Yes, it's only on next (the post-1.0rc3 pipeline). In release_0.8 and earlier, matrix_map is a pointer and the block is only written when it's non-NULL, regardless of zone type - so 5-and-earlier are untouched there. On next, matrix_map became a value member (matrix_map_type matrix_map, RGBControllerInterface.h:427), and GetMatrixMapDescriptionSize (RGBController.cpp:2792) isn't gated by protocol version, so a block is now sent for every zone at any negotiated protocol version (0-5 included). That does change the wire data for the older protocols when talking to a next server - so yes, it's a next-side backwards-compat break. Skipping empty maps (size 0 when h==0 && w==0) on next would restore the old behavior. I have aligned the python parser to key off the size field like the 0.8 client, so it works with both.

[CalcProgrammer1]

Pushed a fix on the OpenRGB side to not send empty maps, but this change still should be implemented on the Python side to correctly match OpenRGB's own parser.

[jath03]

Interesting. I wasn't aware that other zone types could have a matrix map

[CalcProgrammer1]

Interesting. I wasn't aware that other zone types could have a matrix map

In previous versions of OpenRGB the protocol did not exclude the possibility, but I don't know of any controllers that actually did it. On 1.0, I've expanded manually configurable zones (ARGB zones) such that the user has control over the Type field. They can set a matrix map and set the Type field as two independent user options. The matrix map is saved no matter what the type field is set to which allows toggling between matrix and non matrix layouts easily. That is the main place you will see this behavior in practice now.

[CalcProgrammer1]

It also now allows device controllers to build in their own matrix map but make it optional, a keyboard could have an underglow zone that could be switched between mapping into the keyboard matrix or working as an independent light ring.

[septicwolf818]

Servers with SDK protocol >= 5 serialize a matrix map block for every zone, not just MATRIX zones. ZoneData.unpack only consumed it when zone_type == MATRIX, leaving the parser 8 bytes behind on LINEAR/SINGLE zones and producing garbage values (e.g. "256 is not a valid ZoneType") during device discovery.

Consume the matrix block whenever matrix_zone_size > 0, which is backward compatible since the size is 0 when no map exists. Also widen ZoneType to the current server values (LINEAR_LOOP, MATRIX_LOOP_X, MATRIX_LOOP_Y, SEGMENTED) and add a missing fallback so future types don't crash.

[CalcProgrammer1]

Is this only on pipeline? If so it's a bug and needs fixed OpenRGB side, the 5 and earlier protocols should not be changed. If backwards compatibility broke we need to fix it.

[CalcProgrammer1]

Looking at the older OpenRGB code, it actually sends matrix map only if a matrix map exists, not necessarily if type is matrix. It does so happen that 99 if not 100% of controllers that had a non-null matrix map pointer also had ZONE_TYPE_MATRIX, but the old implementation of the server didn't necessarily guarantee this relationship either so in this case I'd say the parser here is in the wrong.

For instance, in the 0.8 release's client side parser, you can see it checks the matrix map size to determine whether or not to parse a matrix map, not the type value:

https://gitlab.com/CalcProgrammer1/OpenRGB/-/blob/release_0.8/RGBController/RGBController.cpp?ref_type=tags#L742

On OpenRGB next (the code that was merged after 1.0rc3 and will soon be released as 1.0), matrix maps are no longer handled as pointers, so every zone "has" a matrix map. I could update the OpenRGB side to not send empty matrix maps (w 0, h 0, map size 0) which would match the old behavior better, save bytes on the network, and reduce breakage caused by this issue. I think I will do this, but this issue should still be fixed here as it was working by happenstance as is.

[septicwolf818]

@CalcProgrammer1 Yes, it's only on next (the post-1.0rc3 pipeline). In release_0.8 and earlier, matrix_map is a pointer and the block is only written when it's non-NULL, regardless of zone type - so 5-and-earlier are untouched there. On next, matrix_map became a value member (matrix_map_type matrix_map, RGBControllerInterface.h:427), and GetMatrixMapDescriptionSize (RGBController.cpp:2792) isn't gated by protocol version, so a block is now sent for every zone at any negotiated protocol version (0-5 included). That does change the wire data for the older protocols when talking to a next server - so yes, it's a next-side backwards-compat break. Skipping empty maps (size 0 when h==0 && w==0) on next would restore the old behavior. I have aligned the python parser to key off the size field like the 0.8 client, so it works with both.

[CalcProgrammer1]

Pushed a fix on the OpenRGB side to not send empty maps, but this change still should be implemented on the Python side to correctly match OpenRGB's own parser.

[jath03]

Interesting. I wasn't aware that other zone types could have a matrix map

[CalcProgrammer1]

Interesting. I wasn't aware that other zone types could have a matrix map

In previous versions of OpenRGB the protocol did not exclude the possibility, but I don't know of any controllers that actually did it. On 1.0, I've expanded manually configurable zones (ARGB zones) such that the user has control over the Type field. They can set a matrix map and set the Type field as two independent user options. The matrix map is saved no matter what the type field is set to which allows toggling between matrix and non matrix layouts easily. That is the main place you will see this behavior in practice now.

[CalcProgrammer1]

It also now allows device controllers to build in their own matrix map but make it optional, a keyboard could have an underglow zone that could be switched between mapping into the keyboard matrix or working as an independent light ring.