Support for DALI memory banks

[ferdinandkeil]

As promised in #66 memory values are now being retrieved using sequences (thanks @sde1000). I also added an automated test for the module using fake gear.

TODO I still need to verify this with my DALI test setup. Right now it's only tested against the fakes module.

PS: please excuse the crude history of the branch. I'm still learning about git rebase 🙄

[sde1000]

Thanks for this! I think it's going the right way.

I think MemoryLocation should represent a contiguous range in a bank, not a single address. This will help in a number of ways:

• It'll save a lot of tedious and possibly error-prone cut-and-paste, and it'll make instances where you don't want to specify a contiguous range much more obvious in the source
• It'll help you write a much more efficient implementation of MemoryValue.retrieve that makes use of the fact that ReadMemoryLocation auto-increments DTR0 until it reaches 0xff (and you should alter your implementation of ReadMemoryLocation in the fake gear class to actually do this) so you don't have to send DTR1() and DTR0() for every location read

I think you should have an object that represents each memory bank. Perhaps define a MemoryBank class as follows:

class MemoryBank:
    def __init__(self, bank, description=None):
        self.bank = bank
        self.description = description

Create an instance of this class for each memory bank that you're declaring values in, and pass that instance to MemoryLocation instead of just the bank number.

Once you've done this, you can extend MemoryLocation to call a method on the MemoryBank to register itself, and the MemoryBank can sanity check that you haven't declared overlapping MemoryLocations within that bank. You might also be able to add a nice introspection interface to MemoryBank so that code reading an entire bank from a device can format the values in it nicely.

[ferdinandkeil]

Thank you for the feedback.

I think MemoryLocation should represent a contiguous range in a bank, not a single address. This will help in a number of ways:

• It'll save a lot of tedious and possibly error-prone cut-and-paste, and it'll make instances where you don't want to specify a contiguous range much more obvious in the source

• It'll help you write a much more efficient implementation of MemoryValue.retrieve that makes use of the fact that ReadMemoryLocation auto-increments DTR0 until it reaches 0xff (and you should alter your implementation of ReadMemoryLocation in the fake gear class to actually do this) so you don't have to send DTR1() and DTR0() for every location read

I tried to avoid copy-and-paste errors by automating the generation of the code. I used the official PDFs as input, copied their tables to a spreadsheet, saved a CSV and then piped that to a short script to automatically write the code.

Declaring ranges would also break the relationship between a memory location and its reset and default value, as these are given separately for every location in the standard.

Regarding your second point: I tried this before but ran into some problems. To me it looked like this was not implemented properly. I confirmed that the TRIDONIC software sends DTR0 and DTR1 every time as well and went on. However, I can check again.

Once you've done this, you can extend MemoryLocation to call a method on the MemoryBank to register itself, and the MemoryBank can sanity check that you haven't declared overlapping MemoryLocations within that bank. You might also be able to add a nice introspection interface to MemoryBank so that code reading an entire bank from a device can format the values in it nicely.

I did something similar in the fakes module, but it's based on inspect. Adding the check for overlapping memory locations should be quite trivial now.

[ferdinandkeil]

I implemented pretty much everything you suggested back in August. The only thing missing is the feature to convert the content of an entire memory bank into formatted values. Would love to hear your feedback,

[sde1000]

I think this is looking good now. I need to spend a bit of time playing around with the test cases. There doesn't appear to be a test of writing to memory banks, or of the "fake" device supporting different sets of memory banks?

[sde1000]

I've had a chance to take a look at this, although my selection of hardware to try it with is currently limited. I've spotted two issues so far:

1. It's repeatedly sending DTR1() while retrieving a memory value. It looks like there's code in MemoryValue.retrieve() to suppress this, but for some reason it's not working. It might be worth adding some code to fakes.Gear to increment a counter whenever a DTRn() command redundantly sets DTRn to its current value so the memory tests can reset this to zero, perform the sequence, and ensure it is no higher than expected. (Presumably "1" for both DTR0 and DTR1 for most values, except where locking is involved?)

2. MemoryValue.retrieve() exits with an exception when trying to read a memory value that is unimplemented on the device. (AttributeError: 'NoneType' object has no attribute 'as_integer'). It should check for this and return None from the sequence instead.

[ferdinandkeil]

These issues should be fixed, but I will continue working on it:

1. The current values for DTR0 and DTR1 are only stored during the retrieval of a single memory value (which can consist of multiple memory locations). They will be resend once another value is requested. I can't think of an elegant solution to this problem.

2. I have added some code to deal with this, but I will also add a test for this feature ASAP.

[sde1000]

I wouldn't worry about keeping track of the values of DTR0 and DTR1 once you leave the sequence: there's no guarantee that they won't be reset by some other bus master in the meantime.

[ferdinandkeil]

[...] there's no guarantee that they won't be reset by some other bus master in the meantime.

Yup, that's what's been keeping me from finding an elegant solution.

I understood that you don't have access to a D4i capable driver. Maybe I can make a donation so you can get one?

[sde1000]

The main issue isn't money, it's finding somewhere that will sell me the kit in the UK!

My last batch of DALI drivers came from https://www.watt24.com/ but I think Brexit has closed that route off.

Can you suggest a device I should be looking for?

[ferdinandkeil]

Well, I've bought my gear from Watt24 as well 😄

This is one of the drivers I have tested: https://www.watt24.com/p/tridonic-led-treiber-lco-40-200-1050-64-o4a-nf-c-exc3/ It's relatively low power, which makes the LED load easier to handle. I also have some 165 W street lighting drivers, but those are beasts. The amount of light their LED loads put out is close to being dangerous.

I wouldn't worry too much about shipping, the big ones (UPS, Fedex, ...) should be able handle the customs stuff just fine.

[ferdinandkeil]

If Watt24 does not manage to send you a driver, I can ship it to you. I can't do anything about customs on your side, but getting it on its way should not be too hard.

[sde1000]

Having a look at the most recent commit da96822d78febb55f0d0fa9c744cf789728f2371:

• You're writing a file called "dali_comm.log" — I suspect this is debug code left in by mistake. (Running the tests shouldn't have side effects.)
• Testing missing memory locations: you're testing by attempting to read LuminaireID, which is a subclass of MemoryValue. Can you add tests for subclasses of NumericValue, ScaledNumericValue, FixedScaleNumericValue, StringValue, BinaryValue and TemperatureValue as well?
• Testing handling of DTR commands: I think this test has missed the point. The idea is to test that the number of DTRx commands issued while reading a sequence of memory locations does not exceed the expected value. You're testing that the first three commands issued by the retrieve() method are DTR1, DTR0 and ReadMemoryLocation.

While I was looking at this I also noticed that LockableValueMixin.is_locked() does not appear to be doing what it says it does, and there are no tests for it. (It appears to yield a DTR1 and DTR0 command then unconditionally return True since a generator object will never be equal to 0x55.)

[ferdinandkeil]

Sorry for missing those :sweat:

• This will be removed with the next commit.
• Will add those.
• LockableValueMixin needs a major overhaul. Will do that and add a test in due course.

[ferdinandkeil]

It looks like you're only checking the first two commands in the sequence, so even if the sequence contains many superfluous DTR0 and DTR1 commands it won't count them.

If the sequence contains superfluous DTR0 or DTR1 commands it will fail either way, as only ReadMemoryLocation commands are allowed after the first two commands. So when reading a value consisting of multiple locations the first two commands have to be DTR0 and DTR1 (order does not matter) followed by several ReadMemoryLocation commands.

[ferdinandkeil]

2. `MemoryValue.retrieve()` exits with an exception when trying to read a memory value that is unimplemented on the device. (`AttributeError: 'NoneType' object has no attribute 'as_integer'`). It should check for this and return None from the sequence instead.

Coming back to this issue: I was thinking whether it might be a better to solution to raise a MemoryLocationNotImplemented (or similarly named) exception when .retrieve() encounters one, forcing the user to deal with this. What do you think?

[sde1000]

I think that's likely to be a good idea, and better than returning None.

Are there better ways to determine which MemoryValues are implemented on a particular device? For example, reading address 0 of the memory bank? Would it be useful to implement a method on MemoryBank that returns the MemoryValues likely to be supported by the device in that bank?

Regarding memory locking: it's my understanding that the lock byte of memory banks exists so that a number of related values can be read from the memory bank that are all snapshots taken at a particular point in time, i.e. the time the lock byte was set to 0x55. Is this correct? If so, I don't think the current API for LockableValueMixin makes sense: the lock byte isn't something we want to read the state of, it's something that we write to to control. We want to be able to set the lock byte to 0x55, read multiple MemoryValues from the bank, then set the lock byte to something else, all in a single sequence (which will be performed by the device driver as a transaction).

[ferdinandkeil]

I think that's likely to be a good idea, and better than returning None.

Done.

Are there better ways to determine which MemoryValues are implemented on a particular device? For example, reading address 0 of the memory bank? Would it be useful to implement a method on MemoryBank that returns the MemoryValues likely to be supported by the device in that bank?

This is doable, however, for the banks specified in DALI parts 251 ff. most fields are mandatory, so it would not make much of a difference.

Regarding memory locking: it's my understanding that the lock byte of memory banks exists so that a number of related values can be read from the memory bank that are all snapshots taken at a particular point in time, i.e. the time the lock byte was set to 0x55. Is this correct? If so, I don't think the current API for LockableValueMixin makes sense: the lock byte isn't something we want to read the state of, it's something that we write to to control. We want to be able to set the lock byte to 0x55, read multiple MemoryValues from the bank, then set the lock byte to something else, all in a single sequence (which will be performed by the device driver as a transaction).

The standard isn't super clear about this. In DALI part 251 ff. it says A manufacturer may provide a vendor-specific means to prevent read and/or write access to individual memory locations. Locations featuring this vendor-specific protection mechanism are marked as: “(protectable)”. Here they call it protectable, in other parts it's called lockable, but I think (hope) it's the same thing. So what I take from this is that the locking mechanism (that in turn changes the value of the lock byte) is vendor-specific and therefore obscure. All that we can do is read whether a value is locked or not.

[sde1000]

Part 102 9.10.2 says:

The byte in location 0x02 shall be used to lock write access. Memory location 0x02 itself shall never be locked for writing. While this memory location contains any value different from 0x55, all memory locations marked "(lockable)" of the corresponding memory bank shall be read only. The control gear shall not change the value of the lock byte other than as a consequence of power cycle or a “RESET MEMORY BANK (DTR0)” or other command affecting the lock byte.

I agree it isn't super clear! (And I was thinking backwards in my previous comment.) And it isn't universal, either: in bank zero, location 0x02 is the number of the last accessible memory bank, not a lock byte.

Part 251 9.2.3 is reasonably clear that setting the lock byte to 0x55 or not only applies to writing to memory banks across the DALI bus, so my previous comment is wrong. I think I was remembering part 252 section 9.2.3 instead:

This standard specifies an additional method that enables latching of a full memory bank. The lock byte is used to control the memory bank latch as follows: If the lock byte contains a value other than 0xAA, writing the following values to the lock byte shall cause the stated result:

• 0xAA: All locations in the memory bank shall be latched and shall not change until the lock byte is written, or a power cycle occurs.
• Other values: The memory bank latch is not affected.

If the lock byte contains the value 0xAA, writing the following values to the lock byte shall cause the stated result:

• 0xAA: All locations in the memory bank shall be re-latched (updated) and shall not change again until the lock byte is written, or a power cycle occurs.
• Other values: The memory bank latch shall be removed. Memory reads shall result in the latest values being returned.

An attempt to write to any location other than the lock byte of a latched memory bank shall result in the same behaviour as if the memory location is not implemented.

Latching of a full memory bank shall not affect reading or writing of other memory banks.

[ferdinandkeil]

This is the first I hear of this latching feature 😓. It does make sense, though. I think I will implement automatic latching where it is necessary (e.g. all time-variable multi-byte values).

One way to deal with the lock byte is to just remove the code handling it and calling it a day. As long as writing memory values is not implemented it's not needed anyway.

[sde1000]

Note that it's not necessary to use the lock byte when reading a single muti-byte value: those are latched automatically. It's only necessary when reading more than one multi-byte value.

The latching behaviour for multi-byte values is described as advisory in part 102 9.10.4 and mandatory in part 252 9.2.1.

Part 102 9.10.4:

To ensure consistent data when reading a multi-byte value from a memory bank, it is recommended that a mechanism be implemented that latches all bytes of the multi-byte value when the first byte of the multi-byte value is read and that unlatches the bytes at any other command than “READ MEMORY LOCATION (DTR1, DTR0)”.

Part 252 9.2.2:

To ensure consistent data when reading a multi-byte value from a memory bank, it is required that a mechanism be implemented that latches all bytes of the multi-byte value when the first byte of the multi-byte value is read and holds them latched until the first byte of any multi-byte or single byte value is read.

[ferdinandkeil]

Added latching as a feature to MemoryBank. The automatic latching specified in the standard should work with the current implementation.

[ferdinandkeil]

I have just tested the current state of the code using an actual driver and it seems to work as exptected. This includes the option to read entire memory banks.

[markus-becker-tridonic-com]

@ferdinandkeil @sde1000 we have used this branch to retrieve D4i data from Tridonic drivers. worked for us. would be great to have this integrated in the main branch.